From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH 1/3] virtio: Basic implementation of virtio pstore driver Date: Tue, 15 Nov 2016 07:06:28 +0200 Message-ID: <20161115065658-mutt-send-email-mst@kernel.org> References: <20160820080744.10344-1-namhyung@kernel.org> <20160820080744.10344-2-namhyung@kernel.org> <20161110182611-mutt-send-email-mst@kernel.org> <20161115045021.GA15992@danjae.aot.lge.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20161115045021.GA15992@danjae.aot.lge.com> 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: Namhyung Kim Cc: virtio-dev@lists.oasis-open.org, Tony Luck , Kees Cook , kvm@vger.kernel.org, Radim =?utf-8?B?S3LEjW3DocWZ?= , Anton Vorontsov , LKML , Steven Rostedt , qemu-devel@nongnu.org, Minchan Kim , Anthony Liguori , Colin Cross , Paolo Bonzini , virtualization@lists.linux-foundation.org, Ingo Molnar List-Id: virtualization@lists.linuxfoundation.org T24gVHVlLCBOb3YgMTUsIDIwMTYgYXQgMDE6NTA6MjFQTSArMDkwMCwgTmFtaHl1bmcgS2ltIHdy b3RlOgo+IEhpIE1pY2hhZWwsCj4gCj4gT24gVGh1LCBOb3YgMTAsIDIwMTYgYXQgMDY6Mzk6NTVQ TSArMDIwMCwgTWljaGFlbCBTLiBUc2lya2luIHdyb3RlOgo+ID4gT24gU2F0LCBBdWcgMjAsIDIw MTYgYXQgMDU6MDc6NDJQTSArMDkwMCwgTmFtaHl1bmcgS2ltIHdyb3RlOgo+ID4gPiBUaGUgdmly dGlvIHBzdG9yZSBkcml2ZXIgcHJvdmlkZXMgaW50ZXJmYWNlIHRvIHRoZSBwc3RvcmUgc3Vic3lz dGVtIHNvCj4gPiA+IHRoYXQgdGhlIGd1ZXN0IGtlcm5lbCdzIGxvZy9kdW1wIG1lc3NhZ2UgY2Fu IGJlIHNhdmVkIG9uIHRoZSBob3N0Cj4gPiA+IG1hY2hpbmUuICBVc2VycyBjYW4gYWNjZXNzIHRo ZSBsb2cgZmlsZSBkaXJlY3RseSBvbiB0aGUgaG9zdCwgb3Igb24gdGhlCj4gPiA+IGd1ZXN0IGF0 IHRoZSBuZXh0IGJvb3QgdXNpbmcgcHN0b3JlIGZpbGVzeXN0ZW0uICBJdCBjdXJyZW50bHkgZGVh bHMgd2l0aAo+ID4gPiBrZXJuZWwgbG9nIChwcmludGspIGJ1ZmZlciBvbmx5LCBidXQgd2UgY2Fu IGV4dGVuZCBpdCB0byBoYXZlIG90aGVyCj4gPiA+IGluZm9ybWF0aW9uIChsaWtlIGZ0cmFjZSBk dW1wKSBsYXRlci4KPiA+ID4gCj4gPiA+IEl0IHN1cHBvcnRzIGxlZ2FjeSBQQ0kgZGV2aWNlIHVz aW5nIHNpbmdsZSBvcmRlci0yIHBhZ2UgYnVmZmVyLgo+ID4gCj4gPiBEbyB5b3UgbWVhbiBhIGxl Z2FjeSB2aXJ0aW8gZGV2aWNlPyBJIGRvbid0IHNlZSB3aHkKPiA+IHlvdSB3b3VsZCB3YW50IHRv IHN1cHBvcnQgcHJlLTEuMCBtb2RlLgo+ID4gSWYgeW91IGRyb3AgdGhhdCwgeW91IGNhbiBkcm9w IGFsbCBjcHVfdG9fdmlydGlvIHRoaW5ncwo+ID4gYW5kIGp1c3QgdXNlIF9fbGUgYWNjZXNzb3Jz Lgo+IAo+IEkgd2FzIHRoaW5raW5nIGFib3V0IHRoZSBrdm10b29scyB3aGljaCBsYWNrcyAxLjAg c3VwcG9ydCBBRkFJSy4KClVubGVzcyBrdm10b29scyB3YW50cyB0byBiZSBsZWZ0IGJlaGluZCBp dCBoYXMgdG8gZ28gMS4wLgoKPiAgQnV0Cj4gSSB0aGluayBpdCdkIGJlIGJldHRlciB0byBhbHdh eXMgdXNlIF9fbGUgdHlwZSBhbnl3YXkuICBXaWxsIGNoYW5nZS4KPiAKPiAKPiA+IAo+ID4gPiBJ dCB1c2VzCj4gPiA+IHR3byB2aXJ0cXVldWVzIC0gb25lIGZvciAoc3luYykgcmVhZCBhbmQgYW5v dGhlciBmb3IgKGFzeW5jKSB3cml0ZS4KPiA+ID4gU2luY2UgaXQgY2Fubm90IHdhaXQgZm9yIHdy aXRlIGZpbmlzaGVkLCBpdCBzdXBwb3J0cyB1cCB0byAxMjgKPiA+ID4gY29uY3VycmVudCBJTy4g IFRoZSBidWZmZXIgc2l6ZSBpcyBjb25maWd1cmFibGUgbm93Lgo+ID4gPiAKPiA+ID4gQ2M6IFBh b2xvIEJvbnppbmkgPHBib256aW5pQHJlZGhhdC5jb20+Cj4gPiA+IENjOiBSYWRpbSBLcsSNbcOh xZkgPHJrcmNtYXJAcmVkaGF0LmNvbT4KPiA+ID4gQ2M6ICJNaWNoYWVsIFMuIFRzaXJraW4iIDxt c3RAcmVkaGF0LmNvbT4KPiA+ID4gQ2M6IEFudGhvbnkgTGlndW9yaSA8YWxpZ3VvcmlAYW1hem9u LmNvbT4KPiA+ID4gQ2M6IEFudG9uIFZvcm9udHNvdiA8YW50b25AZW5vbXNnLm9yZz4KPiA+ID4g Q2M6IENvbGluIENyb3NzIDxjY3Jvc3NAYW5kcm9pZC5jb20+Cj4gPiA+IENjOiBLZWVzIENvb2sg PGtlZXNjb29rQGNocm9taXVtLm9yZz4KPiA+ID4gQ2M6IFRvbnkgTHVjayA8dG9ueS5sdWNrQGlu dGVsLmNvbT4KPiA+ID4gQ2M6IFN0ZXZlbiBSb3N0ZWR0IDxyb3N0ZWR0QGdvb2RtaXMub3JnPgo+ ID4gPiBDYzogSW5nbyBNb2xuYXIgPG1pbmdvQGtlcm5lbC5vcmc+Cj4gPiA+IENjOiBNaW5jaGFu IEtpbSA8bWluY2hhbkBrZXJuZWwub3JnPgo+ID4gPiBDYzoga3ZtQHZnZXIua2VybmVsLm9yZwo+ ID4gPiBDYzogcWVtdS1kZXZlbEBub25nbnUub3JnCj4gPiA+IENjOiB2aXJ0dWFsaXphdGlvbkBs aXN0cy5saW51eC1mb3VuZGF0aW9uLm9yZwo+ID4gPiBTaWduZWQtb2ZmLWJ5OiBOYW1oeXVuZyBL aW0gPG5hbWh5dW5nQGtlcm5lbC5vcmc+Cj4gPiA+IC0tLQo+ID4gPiAgZHJpdmVycy92aXJ0aW8v S2NvbmZpZyAgICAgICAgICAgICB8ICAxMCArCj4gPiA+ICBkcml2ZXJzL3ZpcnRpby9NYWtlZmls ZSAgICAgICAgICAgIHwgICAxICsKPiA+ID4gIGRyaXZlcnMvdmlydGlvL3ZpcnRpb19wc3RvcmUu YyAgICAgfCA0MTcgKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKwo+ID4gPiAg aW5jbHVkZS91YXBpL2xpbnV4L0tidWlsZCAgICAgICAgICB8ICAgMSArCj4gPiA+ICBpbmNsdWRl L3VhcGkvbGludXgvdmlydGlvX2lkcy5oICAgIHwgICAxICsKPiA+ID4gIGluY2x1ZGUvdWFwaS9s aW51eC92aXJ0aW9fcHN0b3JlLmggfCAgNzQgKysrKysrKwo+ID4gPiAgNiBmaWxlcyBjaGFuZ2Vk LCA1MDQgaW5zZXJ0aW9ucygrKQo+ID4gPiAgY3JlYXRlIG1vZGUgMTAwNjQ0IGRyaXZlcnMvdmly dGlvL3ZpcnRpb19wc3RvcmUuYwo+ID4gPiAgY3JlYXRlIG1vZGUgMTAwNjQ0IGluY2x1ZGUvdWFw aS9saW51eC92aXJ0aW9fcHN0b3JlLmgKPiA+ID4gCj4gPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJz L3ZpcnRpby9LY29uZmlnIGIvZHJpdmVycy92aXJ0aW8vS2NvbmZpZwo+ID4gPiBpbmRleCA3NzU5 MDMyMGQ0NGMuLjhmMGU2Yzc5NmMxMiAxMDA2NDQKPiA+ID4gLS0tIGEvZHJpdmVycy92aXJ0aW8v S2NvbmZpZwo+ID4gPiArKysgYi9kcml2ZXJzL3ZpcnRpby9LY29uZmlnCj4gPiA+IEBAIC01OCw2 ICs1OCwxNiBAQCBjb25maWcgVklSVElPX0lOUFVUCj4gPiA+ICAKPiA+ID4gIAkgSWYgdW5zdXJl LCBzYXkgTS4KPiA+ID4gIAo+ID4gPiArY29uZmlnIFZJUlRJT19QU1RPUkUKPiA+ID4gKwl0cmlz dGF0ZSAiVmlydGlvIHBzdG9yZSBkcml2ZXIiCj4gPiA+ICsJZGVwZW5kcyBvbiBWSVJUSU8KPiA+ ID4gKwlkZXBlbmRzIG9uIFBTVE9SRQo+ID4gPiArCS0tLWhlbHAtLS0KPiA+ID4gKwkgVGhpcyBk cml2ZXIgc3VwcG9ydHMgdmlydGlvIHBzdG9yZSBkZXZpY2VzIHRvIHNhdmUvcmVzdG9yZQo+ID4g PiArCSBwYW5pYyBhbmQgb29wcyBtZXNzYWdlcyBvbiB0aGUgaG9zdC4KPiA+ID4gKwo+ID4gPiAr CSBJZiB1bnN1cmUsIHNheSBNLgo+ID4gPiArCj4gPiA+ICAgY29uZmlnIFZJUlRJT19NTUlPCj4g PiA+ICAJdHJpc3RhdGUgIlBsYXRmb3JtIGJ1cyBkcml2ZXIgZm9yIG1lbW9yeSBtYXBwZWQgdmly dGlvIGRldmljZXMiCj4gPiA+ICAJZGVwZW5kcyBvbiBIQVNfSU9NRU0gJiYgSEFTX0RNQQo+ID4g PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy92aXJ0aW8vTWFrZWZpbGUgYi9kcml2ZXJzL3ZpcnRpby9N YWtlZmlsZQo+ID4gPiBpbmRleCA0MWUzMGUzZGM4NDIuLmJlZTY4Y2IyNmQ0OCAxMDA2NDQKPiA+ ID4gLS0tIGEvZHJpdmVycy92aXJ0aW8vTWFrZWZpbGUKPiA+ID4gKysrIGIvZHJpdmVycy92aXJ0 aW8vTWFrZWZpbGUKPiA+ID4gQEAgLTUsMyArNSw0IEBAIHZpcnRpb19wY2kteSA6PSB2aXJ0aW9f cGNpX21vZGVybi5vIHZpcnRpb19wY2lfY29tbW9uLm8KPiA+ID4gIHZpcnRpb19wY2ktJChDT05G SUdfVklSVElPX1BDSV9MRUdBQ1kpICs9IHZpcnRpb19wY2lfbGVnYWN5Lm8KPiA+ID4gIG9iai0k KENPTkZJR19WSVJUSU9fQkFMTE9PTikgKz0gdmlydGlvX2JhbGxvb24ubwo+ID4gPiAgb2JqLSQo Q09ORklHX1ZJUlRJT19JTlBVVCkgKz0gdmlydGlvX2lucHV0Lm8KPiA+ID4gK29iai0kKENPTkZJ R19WSVJUSU9fUFNUT1JFKSArPSB2aXJ0aW9fcHN0b3JlLm8KPiA+ID4gZGlmZiAtLWdpdCBhL2Ry aXZlcnMvdmlydGlvL3ZpcnRpb19wc3RvcmUuYyBiL2RyaXZlcnMvdmlydGlvL3ZpcnRpb19wc3Rv cmUuYwo+ID4gPiBuZXcgZmlsZSBtb2RlIDEwMDY0NAo+ID4gPiBpbmRleCAwMDAwMDAwMDAwMDAu LjBhNjNjN2RiNDI3OAo+ID4gPiAtLS0gL2Rldi9udWxsCj4gPiA+ICsrKyBiL2RyaXZlcnMvdmly dGlvL3ZpcnRpb19wc3RvcmUuYwo+ID4gPiBAQCAtMCwwICsxLDQxNyBAQAo+ID4gPiArI2RlZmlu ZSBwcl9mbXQoZm10KSBLQlVJTERfTU9ETkFNRSAiOiAiIGZtdAo+ID4gPiArCj4gPiA+ICsjaW5j bHVkZSA8bGludXgva2VybmVsLmg+Cj4gPiA+ICsjaW5jbHVkZSA8bGludXgvbW9kdWxlLmg+Cj4g PiA+ICsjaW5jbHVkZSA8bGludXgvcHN0b3JlLmg+Cj4gPiA+ICsjaW5jbHVkZSA8bGludXgvdmly dGlvLmg+Cj4gPiA+ICsjaW5jbHVkZSA8bGludXgvdmlydGlvX2NvbmZpZy5oPgo+ID4gPiArI2lu Y2x1ZGUgPHVhcGkvbGludXgvdmlydGlvX2lkcy5oPgo+ID4gPiArI2luY2x1ZGUgPHVhcGkvbGlu dXgvdmlydGlvX3BzdG9yZS5oPgo+ID4gPiArCj4gPiA+ICsjZGVmaW5lIFZJUlRfUFNUT1JFX09S REVSICAgIDIKPiA+ID4gKyNkZWZpbmUgVklSVF9QU1RPUkVfQlVGU0laRSAgKDQwOTYgPDwgVklS VF9QU1RPUkVfT1JERVIpCj4gPiA+ICsjZGVmaW5lIFZJUlRfUFNUT1JFX05SX1JFUSAgIDEyOAo+ ID4gPiArCj4gPiA+ICtzdHJ1Y3QgdmlydGlvX3BzdG9yZSB7Cj4gPiA+ICsJc3RydWN0IHZpcnRp b19kZXZpY2UJKnZkZXY7Cj4gPiA+ICsJc3RydWN0IHZpcnRxdWV1ZQkqdnFbMl07Cj4gPiAKPiA+ IEknZCBhZGQgbmFtZWQgZmllbGRzIGluc3RlYWQgb2YgYW4gYXJyYXkgaGVyZSwgdnFbMF0KPiA+ IHZxWzFdIGFsbCBvdmVyIHRoZSBwbGFjZSBpcyBoYXJkIHRvIHJlYWQuCj4gCj4gV2lsbCBjaGFu Z2UuCj4gCj4gPiAKPiA+ID4gKwlzdHJ1Y3QgcHN0b3JlX2luZm8JIHBzdG9yZTsKPiA+ID4gKwlz dHJ1Y3QgdmlydGlvX3BzdG9yZV9yZXEgcmVxW1ZJUlRfUFNUT1JFX05SX1JFUV07Cj4gPiA+ICsJ c3RydWN0IHZpcnRpb19wc3RvcmVfcmVzIHJlc1tWSVJUX1BTVE9SRV9OUl9SRVFdOwo+ID4gPiAr CXVuc2lnbmVkIGludAkJIHJlcV9pZDsKPiA+ID4gKwo+ID4gPiArCS8qIFdhaXRpbmcgZm9yIGhv c3QgdG8gYWNrICovCj4gPiA+ICsJd2FpdF9xdWV1ZV9oZWFkX3QJYWNrZWQ7Cj4gPiA+ICsJaW50 CQkJZmFpbGVkOwo+ID4gPiArfTsKPiA+ID4gKwo+ID4gPiArI2RlZmluZSBUWVBFX1RBQkxFX0VO VFJZKF9lbnRyeSkJCQkJXAo+ID4gPiArCXsgUFNUT1JFX1RZUEVfIyNfZW50cnksIFZJUlRJT19Q U1RPUkVfVFlQRV8jI19lbnRyeSB9Cj4gPiA+ICsKPiA+ID4gK3N0cnVjdCB0eXBlX3RhYmxlIHsK PiA+ID4gKwlpbnQgcHN0b3JlOwo+ID4gPiArCXUxNiB2aXJ0aW87Cj4gPiA+ICt9IHR5cGVfdGFi bGVbXSA9IHsKPiA+ID4gKwlUWVBFX1RBQkxFX0VOVFJZKERNRVNHKSwKPiA+ID4gK307Cj4gPiA+ ICsKPiA+ID4gKyN1bmRlZiBUWVBFX1RBQkxFX0VOVFJZCj4gPiAKPiA+IGxldCdzIGF2b2lkIG1h Y3JvcyBmb3Igbm93IHBscy4gSW4gZmFjdCwgSSB3b3VsZCBqdXN0IG9wZW4tY29kZSB0aGlzCj4g PiBpbiB0b192aXJ0aW9fdHlwZSBiZWxvdy4gV2UgY2FuIGFsd2F5cyBjaGFuZ2Ugb3VyIG1pbmRz IGxhdGVyIGlmCj4gPiBsb3RzIG9mIHR5cGVzIGFyZSBhZGRlZC4KPiAKPiBZZXAuCj4gCj4gPiAK PiA+ID4gKwo+ID4gPiArCj4gPiAKPiA+IHNpbmdsZSBlbW90eSBsaW5lIHBscwo+IAo+IE9rLgo+ IAo+ID4gCj4gPiA+ICtzdGF0aWMgdTE2IHRvX3ZpcnRpb190eXBlKHN0cnVjdCB2aXJ0aW9fcHN0 b3JlICp2cHMsIGVudW0gcHN0b3JlX3R5cGVfaWQgdHlwZSkKPiA+ID4gK3sKPiA+ID4gKwl1bnNp Z25lZCBpbnQgaTsKPiA+ID4gKwo+ID4gPiArCWZvciAoaSA9IDA7IGkgPCBBUlJBWV9TSVpFKHR5 cGVfdGFibGUpOyBpKyspIHsKPiA+ID4gKwkJaWYgKHR5cGUgPT0gdHlwZV90YWJsZVtpXS5wc3Rv cmUpCj4gPiA+ICsJCQlyZXR1cm4gY3B1X3RvX3ZpcnRpbzE2KHZwcy0+dmRldiwgdHlwZV90YWJs ZVtpXS52aXJ0aW8pOwo+ID4gPiArCX0KPiA+ID4gKwo+ID4gPiArCXJldHVybiBjcHVfdG9fdmly dGlvMTYodnBzLT52ZGV2LCBWSVJUSU9fUFNUT1JFX1RZUEVfVU5LTk9XTik7Cj4gPiAKPiA+IFRo aXMgYXNzaWducyB1MTYgdG8gX192aXJ0aW8gdHlwZSwgc3BhcnNlIHdpbGwgd2Fybgo+ID4gaWYg eW91IGVuYWJsZSBlbmRpYW4tbmVzcyBjaGVja3MuCj4gPiBQbHMgZml4IHRoYXQgYW5kIGdlbmVy YWxseSwgcGxlYXNlIG1ha2Ugc3VyZSB0aGlzIGlzCj4gPiBjbGVhbiBmcm9tIHNwYXJzZSB3YXJu aW5ncy4KPiAKPiBJJ2xsIHJ1biBzcGFyc2UgYmVmb3JlIHNlbmRpbmcgcGF0Y2ggbmV4dCB0aW1l Lgo+IAo+ID4gCj4gPiA+ICt9Cj4gPiA+ICsKPiA+ID4gK3N0YXRpYyBlbnVtIHBzdG9yZV90eXBl X2lkIGZyb21fdmlydGlvX3R5cGUoc3RydWN0IHZpcnRpb19wc3RvcmUgKnZwcywgdTE2IHR5cGUp Cj4gPiA+ICt7Cj4gPiA+ICsJdW5zaWduZWQgaW50IGk7Cj4gPiA+ICsKPiA+ID4gKwlmb3IgKGkg PSAwOyBpIDwgQVJSQVlfU0laRSh0eXBlX3RhYmxlKTsgaSsrKSB7Cj4gPiA+ICsJCWlmICh2aXJ0 aW8xNl90b19jcHUodnBzLT52ZGV2LCB0eXBlKSA9PSB0eXBlX3RhYmxlW2ldLnZpcnRpbykKPiA+ ID4gKwkJCXJldHVybiB0eXBlX3RhYmxlW2ldLnBzdG9yZTsKPiA+ID4gKwl9Cj4gPiA+ICsKPiA+ ID4gKwlyZXR1cm4gUFNUT1JFX1RZUEVfVU5LTk9XTjsKPiA+ID4gK30KPiA+ID4gKwo+ID4gPiAr c3RhdGljIHZvaWQgdmlydHBzdG9yZV9hY2soc3RydWN0IHZpcnRxdWV1ZSAqdnEpCj4gPiA+ICt7 Cj4gPiA+ICsJc3RydWN0IHZpcnRpb19wc3RvcmUgKnZwcyA9IHZxLT52ZGV2LT5wcml2Owo+ID4g PiArCj4gPiA+ICsJd2FrZV91cCgmdnBzLT5hY2tlZCk7Cj4gPiA+ICt9Cj4gPiA+ICsKPiA+ID4g K3N0YXRpYyB2b2lkIHZpcnRwc3RvcmVfY2hlY2soc3RydWN0IHZpcnRxdWV1ZSAqdnEpCj4gPiA+ ICt7Cj4gPiA+ICsJc3RydWN0IHZpcnRpb19wc3RvcmUgKnZwcyA9IHZxLT52ZGV2LT5wcml2Owo+ ID4gPiArCXN0cnVjdCB2aXJ0aW9fcHN0b3JlX3JlcyAqcmVzOwo+ID4gPiArCXVuc2lnbmVkIGlu dCBsZW47Cj4gPiA+ICsKPiA+ID4gKwlyZXMgPSB2aXJ0cXVldWVfZ2V0X2J1Zih2cSwgJmxlbik7 Cj4gPiA+ICsJaWYgKHJlcyA9PSBOVUxMKQo+ID4gPiArCQlyZXR1cm47Cj4gPiA+ICsKPiA+ID4g KwlpZiAodmlydGlvMzJfdG9fY3B1KHZxLT52ZGV2LCByZXMtPnJldCkgPCAwKQo+ID4gPiArCQl2 cHMtPmZhaWxlZCA9IDE7Cj4gPiA+ICt9Cj4gPiA+ICsKPiA+ID4gK3N0YXRpYyB2b2lkIHZpcnRf cHN0b3JlX2dldF9yZXFzKHN0cnVjdCB2aXJ0aW9fcHN0b3JlICp2cHMsCj4gPiA+ICsJCQkJIHN0 cnVjdCB2aXJ0aW9fcHN0b3JlX3JlcSAqKnByZXEsCj4gPiA+ICsJCQkJIHN0cnVjdCB2aXJ0aW9f cHN0b3JlX3JlcyAqKnByZXMpCj4gPiA+ICt7Cj4gPiA+ICsJdW5zaWduZWQgaW50IGlkeCA9IHZw cy0+cmVxX2lkKysgJSBWSVJUX1BTVE9SRV9OUl9SRVE7Cj4gPiA+ICsKPiA+ID4gKwkqcHJlcSA9 ICZ2cHMtPnJlcVtpZHhdOwo+ID4gPiArCSpwcmVzID0gJnZwcy0+cmVzW2lkeF07Cj4gPiA+ICsK PiA+ID4gKwltZW1zZXQoKnByZXEsIDAsIHNpemVvZigqKnByZXEpKTsKPiA+ID4gKwltZW1zZXQo KnByZXMsIDAsIHNpemVvZigqKnByZXMpKTsKPiA+ID4gK30KPiA+ID4gKwo+ID4gPiArc3RhdGlj IGludCB2aXJ0X3BzdG9yZV9vcGVuKHN0cnVjdCBwc3RvcmVfaW5mbyAqcHNpKQo+ID4gPiArewo+ ID4gPiArCXN0cnVjdCB2aXJ0aW9fcHN0b3JlICp2cHMgPSBwc2ktPmRhdGE7Cj4gPiA+ICsJc3Ry dWN0IHZpcnRpb19wc3RvcmVfcmVxICpyZXE7Cj4gPiA+ICsJc3RydWN0IHZpcnRpb19wc3RvcmVf cmVzICpyZXM7Cj4gPiA+ICsJc3RydWN0IHNjYXR0ZXJsaXN0IHNnb1sxXSwgc2dpWzFdOwo+ID4g PiArCXN0cnVjdCBzY2F0dGVybGlzdCAqc2dzWzJdID0geyBzZ28sIHNnaSB9Owo+ID4gPiArCXVu c2lnbmVkIGludCBsZW47Cj4gPiA+ICsKPiA+ID4gKwl2aXJ0X3BzdG9yZV9nZXRfcmVxcyh2cHMs ICZyZXEsICZyZXMpOwo+ID4gPiArCj4gPiA+ICsJcmVxLT5jbWQgPSBjcHVfdG9fdmlydGlvMTYo dnBzLT52ZGV2LCBWSVJUSU9fUFNUT1JFX0NNRF9PUEVOKTsKPiA+ID4gKwo+ID4gPiArCXNnX2lu aXRfb25lKHNnbywgcmVxLCBzaXplb2YoKnJlcSkpOwo+ID4gPiArCXNnX2luaXRfb25lKHNnaSwg cmVzLCBzaXplb2YoKnJlcykpOwo+ID4gPiArCXZpcnRxdWV1ZV9hZGRfc2dzKHZwcy0+dnFbMF0s IHNncywgMSwgMSwgdnBzLCBHRlBfS0VSTkVMKTsKPiA+ID4gKwl2aXJ0cXVldWVfa2ljayh2cHMt PnZxWzBdKTsKPiA+ID4gKwo+ID4gPiArCXdhaXRfZXZlbnQodnBzLT5hY2tlZCwgdmlydHF1ZXVl X2dldF9idWYodnBzLT52cVswXSwgJmxlbikpOwo+ID4gCj4gPiBEb2VzIHRoaXMgYmxvY2sgdXNl cnNwYWNlIGluIGFuIHVuaW50ZXJydXB0aWJsZSB3YWl0IGlmCj4gPiBoYXJkd2FyZSBpcyBzbG93 PyBUaGF0J3Mgbm90IG5pY2UuCj4gCj4gWWVzLCBidXQgaXQncyBub3QgYSBjb21tb24gb3BlcmF0 aW9uIGFuZCBJIGp1c3Qgd2FudGVkIHRvIG1ha2UgaXQKPiBzaW1wbGUuCj4gCj4gCj4gPiAKPiA+ ID4gKwlyZXR1cm4gdmlydGlvMzJfdG9fY3B1KHZwcy0+dmRldiwgcmVzLT5yZXQpOwo+ID4gPiAr fQo+ID4gPiArCj4gCj4gW1NOSVBdCj4gPiA+ICtzdHJ1Y3QgdmlydGlvX3BzdG9yZV9maWxlaW5m byB7Cj4gPiA+ICsJX192aXJ0aW82NAkJaWQ7Cj4gPiA+ICsJX192aXJ0aW8zMgkJY291bnQ7Cj4g PiA+ICsJX192aXJ0aW8xNgkJdHlwZTsKPiA+ID4gKwlfX3ZpcnRpbzE2CQl1bnVzZWQ7Cj4gPiA+ ICsJX192aXJ0aW8zMgkJZmxhZ3M7Cj4gPiA+ICsJX192aXJ0aW8zMgkJbGVuOwo+ID4gPiArCV9f dmlydGlvNjQJCXRpbWVfc2VjOwo+ID4gPiArCV9fdmlydGlvMzIJCXRpbWVfbnNlYzsKPiA+ID4g KwlfX3ZpcnRpbzMyCQlyZXNlcnZlZDsKPiA+ID4gK307Cj4gPiA+ICsKPiA+ID4gK3N0cnVjdCB2 aXJ0aW9fcHN0b3JlX2NvbmZpZyB7Cj4gPiA+ICsJX192aXJ0aW8zMgkJYnVmc2l6ZTsKPiA+ID4g K307Cj4gPiA+ICsKPiA+IAo+ID4gV2hhdCBleGFjdGx5IGRvZXMgZWFjaCBmaWVsZCBtZWFuPyBJ J20gZXNwZWNpYWxseQo+ID4gaW50ZXJlc3RlZCBpbiB0aW1lIGZpZWxkcyAtIG1haW50YWluaW5n IGEgY29uc2lzdGVudAo+ID4gdGltZSBiZXR3ZWVuIGhvc3QgYW5kIGd1ZXN0IGlzIG5vdCBhIHNp bXBsZSBwcm9ibGVtLgo+IAo+IFRoZXNlIGFyZSByZXF1aXJlZCBieSBwc3RvcmUgYW5kIHdpbGwg YmUgdXNlZCB0byBjcmVhdGUgY29ycmVzcG9uZGluZwo+IGZpbGVzIGluIHRoZSBwc3RvcmUgZmls ZXN5c3RlbS4gIFRoZSB0aW1lIGZpZWxkcyBhcmUgZm9yIG10aW1lIGFuZAo+IGN0aW1lIGFuZCwg SSB0aGluaywgaXQncyBqdXN0IGEgaGludCBmb3IgdXNlciBhbmQgZG9lc24ndCByZXF1aXJlCj4g c3RyaWN0IGNvbnNpc3RlbmN5LgoKUGxzIGFkZCBkb2N1bWVudGF0aW9uLiBJIHdvdWxkIGp1c3Qg ZHJvcCBoaW50cyBmb3Igbm93LgoKPiAKPiBUaGFua3MgZm9yIHlvdXIgcmV2aWV3IQo+IE5hbWh5 dW5nCj4gCj4gPiAKPiA+ID4gKyNlbmRpZiAvKiBfTElOVVhfVklSVElPX1BTVE9SRV9IICovCj4g PiA+IC0tIAo+ID4gPiAyLjkuMwpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fXwpWaXJ0dWFsaXphdGlvbiBtYWlsaW5nIGxpc3QKVmlydHVhbGl6YXRpb25AbGlz dHMubGludXgtZm91bmRhdGlvbi5vcmcKaHR0cHM6Ly9saXN0cy5saW51eGZvdW5kYXRpb24ub3Jn L21haWxtYW4vbGlzdGluZm8vdmlydHVhbGl6YXRpb24= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S941202AbcKOFGe (ORCPT ); Tue, 15 Nov 2016 00:06:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39518 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751803AbcKOFGd (ORCPT ); Tue, 15 Nov 2016 00:06:33 -0500 Date: Tue, 15 Nov 2016 07:06:28 +0200 From: "Michael S. Tsirkin" To: Namhyung Kim Cc: virtio-dev@lists.oasis-open.org, kvm@vger.kernel.org, qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org, LKML , Paolo Bonzini , Radim =?utf-8?B?S3LEjW3DocWZ?= , Anthony Liguori , Anton Vorontsov , Colin Cross , Kees Cook , Tony Luck , Steven Rostedt , Ingo Molnar , Minchan Kim Subject: Re: [PATCH 1/3] virtio: Basic implementation of virtio pstore driver Message-ID: <20161115065658-mutt-send-email-mst@kernel.org> References: <20160820080744.10344-1-namhyung@kernel.org> <20160820080744.10344-2-namhyung@kernel.org> <20161110182611-mutt-send-email-mst@kernel.org> <20161115045021.GA15992@danjae.aot.lge.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20161115045021.GA15992@danjae.aot.lge.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 15 Nov 2016 05:06:32 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Nov 15, 2016 at 01:50:21PM +0900, Namhyung Kim wrote: > Hi Michael, > > On Thu, Nov 10, 2016 at 06:39:55PM +0200, Michael S. Tsirkin wrote: > > On Sat, Aug 20, 2016 at 05:07:42PM +0900, Namhyung Kim wrote: > > > The virtio pstore driver provides interface to the pstore subsystem so > > > that the guest kernel's log/dump message can be saved on the host > > > machine. Users can access the log file directly on the host, or on the > > > guest at the next boot using pstore filesystem. It currently deals with > > > kernel log (printk) buffer only, but we can extend it to have other > > > information (like ftrace dump) later. > > > > > > It supports legacy PCI device using single order-2 page buffer. > > > > Do you mean a legacy virtio device? I don't see why > > you would want to support pre-1.0 mode. > > If you drop that, you can drop all cpu_to_virtio things > > and just use __le accessors. > > I was thinking about the kvmtools which lacks 1.0 support AFAIK. Unless kvmtools wants to be left behind it has to go 1.0. > But > I think it'd be better to always use __le type anyway. Will change. > > > > > > > It uses > > > two virtqueues - one for (sync) read and another for (async) write. > > > Since it cannot wait for write finished, it supports up to 128 > > > concurrent IO. The buffer size is configurable now. > > > > > > Cc: Paolo Bonzini > > > Cc: Radim Krčmář > > > Cc: "Michael S. Tsirkin" > > > Cc: Anthony Liguori > > > Cc: Anton Vorontsov > > > Cc: Colin Cross > > > Cc: Kees Cook > > > Cc: Tony Luck > > > Cc: Steven Rostedt > > > Cc: Ingo Molnar > > > Cc: Minchan Kim > > > Cc: kvm@vger.kernel.org > > > Cc: qemu-devel@nongnu.org > > > Cc: virtualization@lists.linux-foundation.org > > > Signed-off-by: Namhyung Kim > > > --- > > > drivers/virtio/Kconfig | 10 + > > > drivers/virtio/Makefile | 1 + > > > drivers/virtio/virtio_pstore.c | 417 +++++++++++++++++++++++++++++++++++++ > > > include/uapi/linux/Kbuild | 1 + > > > include/uapi/linux/virtio_ids.h | 1 + > > > include/uapi/linux/virtio_pstore.h | 74 +++++++ > > > 6 files changed, 504 insertions(+) > > > create mode 100644 drivers/virtio/virtio_pstore.c > > > create mode 100644 include/uapi/linux/virtio_pstore.h > > > > > > diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig > > > index 77590320d44c..8f0e6c796c12 100644 > > > --- a/drivers/virtio/Kconfig > > > +++ b/drivers/virtio/Kconfig > > > @@ -58,6 +58,16 @@ config VIRTIO_INPUT > > > > > > If unsure, say M. > > > > > > +config VIRTIO_PSTORE > > > + tristate "Virtio pstore driver" > > > + depends on VIRTIO > > > + depends on PSTORE > > > + ---help--- > > > + This driver supports virtio pstore devices to save/restore > > > + panic and oops messages on the host. > > > + > > > + If unsure, say M. > > > + > > > config VIRTIO_MMIO > > > tristate "Platform bus driver for memory mapped virtio devices" > > > depends on HAS_IOMEM && HAS_DMA > > > diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile > > > index 41e30e3dc842..bee68cb26d48 100644 > > > --- a/drivers/virtio/Makefile > > > +++ b/drivers/virtio/Makefile > > > @@ -5,3 +5,4 @@ virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o > > > virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o > > > obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o > > > obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o > > > +obj-$(CONFIG_VIRTIO_PSTORE) += virtio_pstore.o > > > diff --git a/drivers/virtio/virtio_pstore.c b/drivers/virtio/virtio_pstore.c > > > new file mode 100644 > > > index 000000000000..0a63c7db4278 > > > --- /dev/null > > > +++ b/drivers/virtio/virtio_pstore.c > > > @@ -0,0 +1,417 @@ > > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#define VIRT_PSTORE_ORDER 2 > > > +#define VIRT_PSTORE_BUFSIZE (4096 << VIRT_PSTORE_ORDER) > > > +#define VIRT_PSTORE_NR_REQ 128 > > > + > > > +struct virtio_pstore { > > > + struct virtio_device *vdev; > > > + struct virtqueue *vq[2]; > > > > I'd add named fields instead of an array here, vq[0] > > vq[1] all over the place is hard to read. > > Will change. > > > > > > + struct pstore_info pstore; > > > + struct virtio_pstore_req req[VIRT_PSTORE_NR_REQ]; > > > + struct virtio_pstore_res res[VIRT_PSTORE_NR_REQ]; > > > + unsigned int req_id; > > > + > > > + /* Waiting for host to ack */ > > > + wait_queue_head_t acked; > > > + int failed; > > > +}; > > > + > > > +#define TYPE_TABLE_ENTRY(_entry) \ > > > + { PSTORE_TYPE_##_entry, VIRTIO_PSTORE_TYPE_##_entry } > > > + > > > +struct type_table { > > > + int pstore; > > > + u16 virtio; > > > +} type_table[] = { > > > + TYPE_TABLE_ENTRY(DMESG), > > > +}; > > > + > > > +#undef TYPE_TABLE_ENTRY > > > > let's avoid macros for now pls. In fact, I would just open-code this > > in to_virtio_type below. We can always change our minds later if > > lots of types are added. > > Yep. > > > > > > + > > > + > > > > single emoty line pls > > Ok. > > > > > > +static u16 to_virtio_type(struct virtio_pstore *vps, enum pstore_type_id type) > > > +{ > > > + unsigned int i; > > > + > > > + for (i = 0; i < ARRAY_SIZE(type_table); i++) { > > > + if (type == type_table[i].pstore) > > > + return cpu_to_virtio16(vps->vdev, type_table[i].virtio); > > > + } > > > + > > > + return cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_TYPE_UNKNOWN); > > > > This assigns u16 to __virtio type, sparse will warn > > if you enable endian-ness checks. > > Pls fix that and generally, please make sure this is > > clean from sparse warnings. > > I'll run sparse before sending patch next time. > > > > > > +} > > > + > > > +static enum pstore_type_id from_virtio_type(struct virtio_pstore *vps, u16 type) > > > +{ > > > + unsigned int i; > > > + > > > + for (i = 0; i < ARRAY_SIZE(type_table); i++) { > > > + if (virtio16_to_cpu(vps->vdev, type) == type_table[i].virtio) > > > + return type_table[i].pstore; > > > + } > > > + > > > + return PSTORE_TYPE_UNKNOWN; > > > +} > > > + > > > +static void virtpstore_ack(struct virtqueue *vq) > > > +{ > > > + struct virtio_pstore *vps = vq->vdev->priv; > > > + > > > + wake_up(&vps->acked); > > > +} > > > + > > > +static void virtpstore_check(struct virtqueue *vq) > > > +{ > > > + struct virtio_pstore *vps = vq->vdev->priv; > > > + struct virtio_pstore_res *res; > > > + unsigned int len; > > > + > > > + res = virtqueue_get_buf(vq, &len); > > > + if (res == NULL) > > > + return; > > > + > > > + if (virtio32_to_cpu(vq->vdev, res->ret) < 0) > > > + vps->failed = 1; > > > +} > > > + > > > +static void virt_pstore_get_reqs(struct virtio_pstore *vps, > > > + struct virtio_pstore_req **preq, > > > + struct virtio_pstore_res **pres) > > > +{ > > > + unsigned int idx = vps->req_id++ % VIRT_PSTORE_NR_REQ; > > > + > > > + *preq = &vps->req[idx]; > > > + *pres = &vps->res[idx]; > > > + > > > + memset(*preq, 0, sizeof(**preq)); > > > + memset(*pres, 0, sizeof(**pres)); > > > +} > > > + > > > +static int virt_pstore_open(struct pstore_info *psi) > > > +{ > > > + struct virtio_pstore *vps = psi->data; > > > + struct virtio_pstore_req *req; > > > + struct virtio_pstore_res *res; > > > + struct scatterlist sgo[1], sgi[1]; > > > + struct scatterlist *sgs[2] = { sgo, sgi }; > > > + unsigned int len; > > > + > > > + virt_pstore_get_reqs(vps, &req, &res); > > > + > > > + req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_OPEN); > > > + > > > + sg_init_one(sgo, req, sizeof(*req)); > > > + sg_init_one(sgi, res, sizeof(*res)); > > > + virtqueue_add_sgs(vps->vq[0], sgs, 1, 1, vps, GFP_KERNEL); > > > + virtqueue_kick(vps->vq[0]); > > > + > > > + wait_event(vps->acked, virtqueue_get_buf(vps->vq[0], &len)); > > > > Does this block userspace in an uninterruptible wait if > > hardware is slow? That's not nice. > > Yes, but it's not a common operation and I just wanted to make it > simple. > > > > > > > + return virtio32_to_cpu(vps->vdev, res->ret); > > > +} > > > + > > [SNIP] > > > +struct virtio_pstore_fileinfo { > > > + __virtio64 id; > > > + __virtio32 count; > > > + __virtio16 type; > > > + __virtio16 unused; > > > + __virtio32 flags; > > > + __virtio32 len; > > > + __virtio64 time_sec; > > > + __virtio32 time_nsec; > > > + __virtio32 reserved; > > > +}; > > > + > > > +struct virtio_pstore_config { > > > + __virtio32 bufsize; > > > +}; > > > + > > > > What exactly does each field mean? I'm especially > > interested in time fields - maintaining a consistent > > time between host and guest is not a simple problem. > > These are required by pstore and will be used to create corresponding > files in the pstore filesystem. The time fields are for mtime and > ctime and, I think, it's just a hint for user and doesn't require > strict consistency. Pls add documentation. I would just drop hints for now. > > Thanks for your review! > Namhyung > > > > > > +#endif /* _LINUX_VIRTIO_PSTORE_H */ > > > -- > > > 2.9.3 From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57237) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c6Vx6-0003CA-Li for qemu-devel@nongnu.org; Tue, 15 Nov 2016 00:06:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c6Vx3-0008Di-Eq for qemu-devel@nongnu.org; Tue, 15 Nov 2016 00:06:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42676) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c6Vx3-0008Db-6X for qemu-devel@nongnu.org; Tue, 15 Nov 2016 00:06:33 -0500 Date: Tue, 15 Nov 2016 07:06:28 +0200 From: "Michael S. Tsirkin" Message-ID: <20161115065658-mutt-send-email-mst@kernel.org> References: <20160820080744.10344-1-namhyung@kernel.org> <20160820080744.10344-2-namhyung@kernel.org> <20161110182611-mutt-send-email-mst@kernel.org> <20161115045021.GA15992@danjae.aot.lge.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20161115045021.GA15992@danjae.aot.lge.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/3] virtio: Basic implementation of virtio pstore driver List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Namhyung Kim Cc: virtio-dev@lists.oasis-open.org, kvm@vger.kernel.org, qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org, LKML , Paolo Bonzini , Radim =?utf-8?B?S3LEjW3DocWZ?= , Anthony Liguori , Anton Vorontsov , Colin Cross , Kees Cook , Tony Luck , Steven Rostedt , Ingo Molnar , Minchan Kim On Tue, Nov 15, 2016 at 01:50:21PM +0900, Namhyung Kim wrote: > Hi Michael, >=20 > On Thu, Nov 10, 2016 at 06:39:55PM +0200, Michael S. Tsirkin wrote: > > On Sat, Aug 20, 2016 at 05:07:42PM +0900, Namhyung Kim wrote: > > > The virtio pstore driver provides interface to the pstore subsystem= so > > > that the guest kernel's log/dump message can be saved on the host > > > machine. Users can access the log file directly on the host, or on= the > > > guest at the next boot using pstore filesystem. It currently deals= with > > > kernel log (printk) buffer only, but we can extend it to have other > > > information (like ftrace dump) later. > > >=20 > > > It supports legacy PCI device using single order-2 page buffer. > >=20 > > Do you mean a legacy virtio device? I don't see why > > you would want to support pre-1.0 mode. > > If you drop that, you can drop all cpu_to_virtio things > > and just use __le accessors. >=20 > I was thinking about the kvmtools which lacks 1.0 support AFAIK. Unless kvmtools wants to be left behind it has to go 1.0. > But > I think it'd be better to always use __le type anyway. Will change. >=20 >=20 > >=20 > > > It uses > > > two virtqueues - one for (sync) read and another for (async) write. > > > Since it cannot wait for write finished, it supports up to 128 > > > concurrent IO. The buffer size is configurable now. > > >=20 > > > Cc: Paolo Bonzini > > > Cc: Radim Kr=C4=8Dm=C3=A1=C5=99 > > > Cc: "Michael S. Tsirkin" > > > Cc: Anthony Liguori > > > Cc: Anton Vorontsov > > > Cc: Colin Cross > > > Cc: Kees Cook > > > Cc: Tony Luck > > > Cc: Steven Rostedt > > > Cc: Ingo Molnar > > > Cc: Minchan Kim > > > Cc: kvm@vger.kernel.org > > > Cc: qemu-devel@nongnu.org > > > Cc: virtualization@lists.linux-foundation.org > > > Signed-off-by: Namhyung Kim > > > --- > > > drivers/virtio/Kconfig | 10 + > > > drivers/virtio/Makefile | 1 + > > > drivers/virtio/virtio_pstore.c | 417 +++++++++++++++++++++++++= ++++++++++++ > > > include/uapi/linux/Kbuild | 1 + > > > include/uapi/linux/virtio_ids.h | 1 + > > > include/uapi/linux/virtio_pstore.h | 74 +++++++ > > > 6 files changed, 504 insertions(+) > > > create mode 100644 drivers/virtio/virtio_pstore.c > > > create mode 100644 include/uapi/linux/virtio_pstore.h > > >=20 > > > diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig > > > index 77590320d44c..8f0e6c796c12 100644 > > > --- a/drivers/virtio/Kconfig > > > +++ b/drivers/virtio/Kconfig > > > @@ -58,6 +58,16 @@ config VIRTIO_INPUT > > > =20 > > > If unsure, say M. > > > =20 > > > +config VIRTIO_PSTORE > > > + tristate "Virtio pstore driver" > > > + depends on VIRTIO > > > + depends on PSTORE > > > + ---help--- > > > + This driver supports virtio pstore devices to save/restore > > > + panic and oops messages on the host. > > > + > > > + If unsure, say M. > > > + > > > config VIRTIO_MMIO > > > tristate "Platform bus driver for memory mapped virtio devices" > > > depends on HAS_IOMEM && HAS_DMA > > > diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile > > > index 41e30e3dc842..bee68cb26d48 100644 > > > --- a/drivers/virtio/Makefile > > > +++ b/drivers/virtio/Makefile > > > @@ -5,3 +5,4 @@ virtio_pci-y :=3D virtio_pci_modern.o virtio_pci_co= mmon.o > > > virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) +=3D virtio_pci_legacy.o > > > obj-$(CONFIG_VIRTIO_BALLOON) +=3D virtio_balloon.o > > > obj-$(CONFIG_VIRTIO_INPUT) +=3D virtio_input.o > > > +obj-$(CONFIG_VIRTIO_PSTORE) +=3D virtio_pstore.o > > > diff --git a/drivers/virtio/virtio_pstore.c b/drivers/virtio/virtio= _pstore.c > > > new file mode 100644 > > > index 000000000000..0a63c7db4278 > > > --- /dev/null > > > +++ b/drivers/virtio/virtio_pstore.c > > > @@ -0,0 +1,417 @@ > > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#define VIRT_PSTORE_ORDER 2 > > > +#define VIRT_PSTORE_BUFSIZE (4096 << VIRT_PSTORE_ORDER) > > > +#define VIRT_PSTORE_NR_REQ 128 > > > + > > > +struct virtio_pstore { > > > + struct virtio_device *vdev; > > > + struct virtqueue *vq[2]; > >=20 > > I'd add named fields instead of an array here, vq[0] > > vq[1] all over the place is hard to read. >=20 > Will change. >=20 > >=20 > > > + struct pstore_info pstore; > > > + struct virtio_pstore_req req[VIRT_PSTORE_NR_REQ]; > > > + struct virtio_pstore_res res[VIRT_PSTORE_NR_REQ]; > > > + unsigned int req_id; > > > + > > > + /* Waiting for host to ack */ > > > + wait_queue_head_t acked; > > > + int failed; > > > +}; > > > + > > > +#define TYPE_TABLE_ENTRY(_entry) \ > > > + { PSTORE_TYPE_##_entry, VIRTIO_PSTORE_TYPE_##_entry } > > > + > > > +struct type_table { > > > + int pstore; > > > + u16 virtio; > > > +} type_table[] =3D { > > > + TYPE_TABLE_ENTRY(DMESG), > > > +}; > > > + > > > +#undef TYPE_TABLE_ENTRY > >=20 > > let's avoid macros for now pls. In fact, I would just open-code this > > in to_virtio_type below. We can always change our minds later if > > lots of types are added. >=20 > Yep. >=20 > >=20 > > > + > > > + > >=20 > > single emoty line pls >=20 > Ok. >=20 > >=20 > > > +static u16 to_virtio_type(struct virtio_pstore *vps, enum pstore_t= ype_id type) > > > +{ > > > + unsigned int i; > > > + > > > + for (i =3D 0; i < ARRAY_SIZE(type_table); i++) { > > > + if (type =3D=3D type_table[i].pstore) > > > + return cpu_to_virtio16(vps->vdev, type_table[i].virtio); > > > + } > > > + > > > + return cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_TYPE_UNKNOWN); > >=20 > > This assigns u16 to __virtio type, sparse will warn > > if you enable endian-ness checks. > > Pls fix that and generally, please make sure this is > > clean from sparse warnings. >=20 > I'll run sparse before sending patch next time. >=20 > >=20 > > > +} > > > + > > > +static enum pstore_type_id from_virtio_type(struct virtio_pstore *= vps, u16 type) > > > +{ > > > + unsigned int i; > > > + > > > + for (i =3D 0; i < ARRAY_SIZE(type_table); i++) { > > > + if (virtio16_to_cpu(vps->vdev, type) =3D=3D type_table[i].virtio= ) > > > + return type_table[i].pstore; > > > + } > > > + > > > + return PSTORE_TYPE_UNKNOWN; > > > +} > > > + > > > +static void virtpstore_ack(struct virtqueue *vq) > > > +{ > > > + struct virtio_pstore *vps =3D vq->vdev->priv; > > > + > > > + wake_up(&vps->acked); > > > +} > > > + > > > +static void virtpstore_check(struct virtqueue *vq) > > > +{ > > > + struct virtio_pstore *vps =3D vq->vdev->priv; > > > + struct virtio_pstore_res *res; > > > + unsigned int len; > > > + > > > + res =3D virtqueue_get_buf(vq, &len); > > > + if (res =3D=3D NULL) > > > + return; > > > + > > > + if (virtio32_to_cpu(vq->vdev, res->ret) < 0) > > > + vps->failed =3D 1; > > > +} > > > + > > > +static void virt_pstore_get_reqs(struct virtio_pstore *vps, > > > + struct virtio_pstore_req **preq, > > > + struct virtio_pstore_res **pres) > > > +{ > > > + unsigned int idx =3D vps->req_id++ % VIRT_PSTORE_NR_REQ; > > > + > > > + *preq =3D &vps->req[idx]; > > > + *pres =3D &vps->res[idx]; > > > + > > > + memset(*preq, 0, sizeof(**preq)); > > > + memset(*pres, 0, sizeof(**pres)); > > > +} > > > + > > > +static int virt_pstore_open(struct pstore_info *psi) > > > +{ > > > + struct virtio_pstore *vps =3D psi->data; > > > + struct virtio_pstore_req *req; > > > + struct virtio_pstore_res *res; > > > + struct scatterlist sgo[1], sgi[1]; > > > + struct scatterlist *sgs[2] =3D { sgo, sgi }; > > > + unsigned int len; > > > + > > > + virt_pstore_get_reqs(vps, &req, &res); > > > + > > > + req->cmd =3D cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_OPEN); > > > + > > > + sg_init_one(sgo, req, sizeof(*req)); > > > + sg_init_one(sgi, res, sizeof(*res)); > > > + virtqueue_add_sgs(vps->vq[0], sgs, 1, 1, vps, GFP_KERNEL); > > > + virtqueue_kick(vps->vq[0]); > > > + > > > + wait_event(vps->acked, virtqueue_get_buf(vps->vq[0], &len)); > >=20 > > Does this block userspace in an uninterruptible wait if > > hardware is slow? That's not nice. >=20 > Yes, but it's not a common operation and I just wanted to make it > simple. >=20 >=20 > >=20 > > > + return virtio32_to_cpu(vps->vdev, res->ret); > > > +} > > > + >=20 > [SNIP] > > > +struct virtio_pstore_fileinfo { > > > + __virtio64 id; > > > + __virtio32 count; > > > + __virtio16 type; > > > + __virtio16 unused; > > > + __virtio32 flags; > > > + __virtio32 len; > > > + __virtio64 time_sec; > > > + __virtio32 time_nsec; > > > + __virtio32 reserved; > > > +}; > > > + > > > +struct virtio_pstore_config { > > > + __virtio32 bufsize; > > > +}; > > > + > >=20 > > What exactly does each field mean? I'm especially > > interested in time fields - maintaining a consistent > > time between host and guest is not a simple problem. >=20 > These are required by pstore and will be used to create corresponding > files in the pstore filesystem. The time fields are for mtime and > ctime and, I think, it's just a hint for user and doesn't require > strict consistency. Pls add documentation. I would just drop hints for now. >=20 > Thanks for your review! > Namhyung >=20 > >=20 > > > +#endif /* _LINUX_VIRTIO_PSTORE_H */ > > > --=20 > > > 2.9.3