From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexey G Subject: Re: [RFC PATCH 16/30] q35/xen: Add Xen platform device support for Q35 Date: Wed, 14 Mar 2018 09:49:19 +1000 Message-ID: <20180314094919.00004965@gmail.com> References: <4a65e8b30fe9d2a6c1c53b85ef2697f02e01d13f.1520867956.git.x1917x@gmail.com> <20180312194406.GX3417@localhost.localdomain> <20180313065637.00005cee@gmail.com> <20180312214402.GY3417@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from us1-rack-dfw2.inumbo.com ([104.130.134.6]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1evtff-00023W-Tm for xen-devel@lists.xenproject.org; Tue, 13 Mar 2018 23:49:31 +0000 Received: by mail-lf0-x244.google.com with SMTP id w16-v6so1998269lfc.13 for ; Tue, 13 Mar 2018 16:49:29 -0700 (PDT) In-Reply-To: <20180312214402.GY3417@localhost.localdomain> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" To: Eduardo Habkost Cc: "Michael S. Tsirkin" , qemu-devel@nongnu.org, Paolo Bonzini , Marcel Apfelbaum , xen-devel@lists.xenproject.org, Richard Henderson List-Id: xen-devel@lists.xenproject.org T24gTW9uLCAxMiBNYXIgMjAxOCAxODo0NDowMiAtMDMwMApFZHVhcmRvIEhhYmtvc3QgPGVoYWJr b3N0QHJlZGhhdC5jb20+IHdyb3RlOgoKPk9uIFR1ZSwgTWFyIDEzLCAyMDE4IGF0IDA2OjU2OjM3 QU0gKzEwMDAsIEFsZXhleSBHIHdyb3RlOgo+PiBPbiBNb24sIDEyIE1hciAyMDE4IDE2OjQ0OjA2 IC0wMzAwCj4+IEVkdWFyZG8gSGFia29zdCA8ZWhhYmtvc3RAcmVkaGF0LmNvbT4gd3JvdGU6Cj4+ ICAgCj4+ID5PbiBUdWUsIE1hciAxMywgMjAxOCBhdCAwNDozNDowMUFNICsxMDAwLCBBbGV4ZXkg R2VyYXNpbWVua28KPj4gPndyb3RlOiAgCj4+ID4+IEN1cnJlbnQgWGVuL1FFTVUgbWV0aG9kIHRv IGNvbnRyb2wgWGVuIFBsYXRmb3JtIGRldmljZSBvbiBpNDQwIGlzCj4+ID4+IGEgYml0IG9kZCAt LSBlbmFibGluZy9kaXNhYmxpbmcgWGVuIHBsYXRmb3JtIGRldmljZSBhY3R1YWxseQo+PiA+PiBt b2RpZmllcyB0aGUgUUVNVSBlbXVsYXRlZCBtYWNoaW5lIHR5cGUsIG5hbWVseSB4ZW5mdiA8LS0+ IHBjLgo+PiA+PiAKPj4gPj4gSW4gb3JkZXIgdG8gYXZvaWQgbXVsdGlwbHlpbmcgbWFjaGluZSB0 eXBlcywgdXNlIGEgbmV3IHdheSB0bwo+PiA+PiBjb250cm9sIFhlbiBQbGF0Zm9ybSBkZXZpY2Ug Zm9yIFFFTVUgLS0gInhlbi1wbGF0Zm9ybS1kZXYiIG1hY2hpbmUKPj4gPj4gcHJvcGVydHkgKGJv b2wpLiBUbyBtYWludGFpbiBiYWNrd2FyZCBjb21wYXRpYmlsaXR5IHdpdGggZXhpc3RpbmcKPj4g Pj4gWGVuL1FFTVUgc2V0dXBzLCB0aGlzIGlzIG9ubHkgYXBwbGljYWJsZSB0byBxMzUgbWFjaGlu ZSBjdXJyZW50bHkuCj4+ID4+IGk0NDAgZW11bGF0aW9uIHN0aWxsIHVzZXMgdGhlIG9sZCBtZXRo b2QgKGkuZS4geGVuZnYvcGMgbWFjaGluZQo+PiA+PiBzZWxlY3Rpb24pIHRvIGNvbnRyb2wgWGVu IFBsYXRmb3JtIGRldmljZSwgdGhpcyBtYXkgYmUgY2hhbmdlZAo+PiA+PiBsYXRlciB0byB4ZW4t cGxhdGZvcm0tZGV2IHByb3BlcnR5IGFzIHdlbGwuCj4+ID4+IAo+PiA+PiBUaGlzIHdheSB3ZSBj YW4gdXNlIGEgc2luZ2xlIG1hY2hpbmUgdHlwZSAocTM1KSBhbmQgY2hhbmdlIGp1c3QKPj4gPj4g eGVuLXBsYXRmb3JtLWRldiB2YWx1ZSB0byBvbi9vZmYgdG8gY29udHJvbCBYZW4gcGxhdGZvcm0g ZGV2aWNlLgo+PiA+PiAKPj4gPj4gU2lnbmVkLW9mZi1ieTogQWxleGV5IEdlcmFzaW1lbmtvIDx4 MTkxN3hAZ21haWwuY29tPgo+PiA+PiAtLS0gICAgCj4+ID5bLi4uXSAgCj4+ID4+IGRpZmYgLS1n aXQgYS9xZW11LW9wdGlvbnMuaHggYi9xZW11LW9wdGlvbnMuaHgKPj4gPj4gaW5kZXggNjU4NTA1 OGM2Yy4uY2VlMGI5MjAyOCAxMDA2NDQKPj4gPj4gLS0tIGEvcWVtdS1vcHRpb25zLmh4Cj4+ID4+ ICsrKyBiL3FlbXUtb3B0aW9ucy5oeAo+PiA+PiBAQCAtMzgsNiArMzgsNyBAQCBERUYoIm1hY2hp bmUiLCBIQVNfQVJHLCBRRU1VX09QVElPTl9tYWNoaW5lLCBcCj4+ID4+ICAgICAgIiAgICAgICAg ICAgICAgICBkdW1wLWd1ZXN0LWNvcmU9b258b2ZmIGluY2x1ZGUgZ3Vlc3QgbWVtb3J5Cj4+ID4+ IGluIGEgY29yZSBkdW1wIChkZWZhdWx0PW9uKVxuIiAiICAgICAgICAgICAgICAgIG1lbS1tZXJn ZT1vbnxvZmYKPj4gPj4gY29udHJvbHMgbWVtb3J5IG1lcmdlIHN1cHBvcnQgKGRlZmF1bHQ6IG9u KVxuIiAiCj4+ID4+IGlnZC1wYXNzdGhydT1vbnxvZmYgY29udHJvbHMgSUdEIEdGWCBwYXNzdGhy b3VnaCBzdXBwb3J0Cj4+ID4+IChkZWZhdWx0PW9mZilcbiIKPj4gPj4gKyAgICAiICAgICAgICAg ICAgICAgIHhlbi1wbGF0Zm9ybS1kZXY9b258b2ZmIGNvbnRyb2xzIFhlbgo+PiA+PiBQbGF0Zm9y bSBkZXZpY2UgKGRlZmF1bHQ9b2ZmKVxuIiAiCj4+ID4+IGFlcy1rZXktd3JhcD1vbnxvZmYgY29u dHJvbHMgc3VwcG9ydCBmb3IgQUVTIGtleSB3cmFwcGluZwo+PiA+PiAoZGVmYXVsdD1vbilcbiIg IiAgICAgICAgICAgICAgICBkZWEta2V5LXdyYXA9b258b2ZmIGNvbnRyb2xzCj4+ID4+IHN1cHBv cnQgZm9yIERFQSBrZXkgd3JhcHBpbmcgKGRlZmF1bHQ9b24pXG4iICIKPj4gPj4gc3VwcHJlc3Mt dm1kZXNjPW9ufG9mZiBkaXNhYmxlcyBzZWxmLWRlc2NyaWJpbmcgbWlncmF0aW9uCj4+ID4+IChk ZWZhdWx0PW9mZilcbiIgICAgCj4+ID4KPj4gPldoYXQgYXJlIHRoZSBvYnN0YWNsZXMgcHJldmVu dGluZyAiLWRldmljZSB4ZW4tcGxhdGZvcm0iIGZyb20KPj4gPndvcmtpbmc/ICBJdCB3b3VsZCBi ZSBiZXR0ZXIgdGhhbiBhZGRpbmcgYSBuZXcgYm9vbGVhbiBvcHRpb24gdG8KPj4gPi1tYWNoaW5l LiAgCj4+IAo+PiBJIGd1ZXNzIHRoZSBpbml0aWFsIGFzc3VtcHRpb24gd2FzIHRoYXQgY2hhbmdp bmcgdGhlCj4+IHhlbl9wbGF0Zm9ybV9kZXZpY2UgdmFsdWUgaW4gWGVuJ3Mgb3B0aW9ucyBtYXkg Y2F1c2Ugc29tZSBhZGRpdGlvbmFsCj4+IGNoYW5nZXMgaW4gcGxhdGZvcm0gY29uZmlndXJhdGlv biBiZXNpZGVzIGFkZGluZyAob3Igbm90KSB0aGUgWGVuCj4+IFBsYXRmb3JtIGRldmljZSwgaGVu Y2UgYSBjb21wbGV0ZWx5IGRpZmZlcmVudCBtYWNoaW5lIHR5cGUgd2FzIGNob3Nlbgo+PiAoeGVu ZnYpLgo+PiAKPj4gQXQgdGhlIG1vbWVudCBwYyxhY2NlbD14ZW4veGVuZnYgc2VsZWN0aW9uIG1v c3RseSBnb3Zlcm5zCj4+IG9ubHkgdGhlIFhlbiBQbGF0Zm9ybSBkZXZpY2UgcHJlc2VuY2UuIEFs c28gc2V0dGluZyBtYXhfY3B1cyB0bwo+PiBIVk1fTUFYX1ZDUFVTIGRlcGVuZHMgb24gaXQsIGJ1 dCB0aGlzIGRvZXNuJ3QgYXBwbGljYWJsZSB0byBhCj4+ICdwYyxhY2NlbD14ZW4nIG1hY2hpbmUg Zm9yIHNvbWUgcmVhc29uLgo+PiAKPj4gSWYgYXBwbHlpbmcgSFZNX01BWF9WQ1BVUyB0byBtYXhf Y3B1cyBpcyByZWFsbHkgbmVjZXNzYXJ5IEkgdGhpbmsKPj4gaXQncyBiZXR0ZXIgdG8gc2V0IGl0 IHVuY29uZGl0aW9uYWxseSBmb3IgYWxsICdhY2NlbD14ZW4nIEhWTSBtYWNoaW5lCj4+IHR5cGVz IGluc2lkZSB4ZW5fZW5hYmxlZCgpIGJsb2NrLiBSaWdodCBub3cgaXQncyBtaXNzaW5nIGZvcgo+ PiBwYyxhY2NlbD14ZW4gYW5kIHEzNSxhY2NlbD14ZW4uICAKPgo+SWYgeW91IGFyZSB0YWxraW5n IGFib3V0IE1hY2hpbmVDbGFzczo6bWF4X2NwdXMsIG5vdGUgdGhhdCBpdCBpcwo+cmV0dXJuZWQg YnkgcXVlcnktbWFjaGluZXMsIHNvIGl0J3Mgc3VwcG9zZWQgdG8gYmUgYSBzdGF0aWMKPnZhbHVl LiAgQ2hhbmdpbmcgaXQgYSBydW50aW1lIHdvdWxkIG1lYW4gdGhlIHF1ZXJ5LW1hY2hpbmVzIHZh bHVlCj5pcyBpbmNvcnJlY3QuCj4KPklzIEhWTV9NQVhfQ1BVUyBoaWdoZXIgb3IgbG93ZXIgdGhh biAyNTU/ICBJZiBpdCdzIGhpZ2hlciwgZG9lcwo+aXQgbWVhbiB0aGUgY3VycmVudCB2YWx1ZSBv biBwYyBhbmQgcTM1IGlzbid0IGFjY3VyYXRlPwoKSFZNX01BWF9WQ1BVUyBpcyAxMjggY3VycmVu dGx5LCBidXQgdGhlcmUgaXMgYW4gb25nb2luZyB3b3JrIGZyb20gSW50ZWwKdG8gc3VwcG9ydCBt b3JlIHZjcHVzIGFuZCA+OGJpdCBBUElDIElEcywgc28gdGhpcyBudW1iZXIgd2lsbCBsaWtlbHkK Y2hhbmdlIHNvb24uCgpBY2NvcmRpbmcgdG8gdGhlIGNvZGUsIHVzaW5nIEhWTV9NQVhfVkNQVVMg aW4gUUVNVSBpcyBhIGJpdCBleGNlc3NpdmUgYXMKdGhlIG1heGltdW0gbnVtYmVyIG9mIHZjcHVz IGlzIGNvbnRyb2xsZWQgb24gWGVuIHNpZGUgYW55d2F5LiBDdXJyZW50bHkKSFZNX01BWF9WQ1BV UyBpcyB1c2VkIGluIGEgb25lLXRpbWUgY2hlY2sgZm9yIHRoZSBtYXhjcHVzIHZhbHVlICh3aGlj aAppdHNlbGYgY29tZXMgZnJvbSBsaWJ4bCkuCkkgdGhpbmsgZm9yIGZ1dHVyZSBjb21wYXRpYmls aXR5IGl0J3MgYmV0dGVyIHRvIHNldCBtYy0+bWF4X2NwdXMgdG8KSFZNX01BWF9WQ1BVUyBmb3Ig YWxsIGFjY2VsPXhlbiBIVk0tc3VwcG9ydGVkIG1hY2hpbmUgdHlwZXMsIG5vdCBqdXN0CnhlbmZ2 LgoKVGhlICctZGV2aWNlJyBhcHByb2FjaCB5b3Ugc3VnZ2VzdGVkIHNlZW1zIG1vcmUgcHJlZmVy YWJsZSB0aGFuIGEKbWFjaGluZSBib29sIHByb3BlcnR5LCBJJ2xsIHRyeSBzd2l0Y2hpbmcgdG8g aXQuCgo+SXMgSFZNX01BWF9DUFVTIHNvbWV0aGluZyB0aGF0IG5lZWRzIHRvIGJlIGVuYWJsZWQg YmVjYXVzZSBvZgo+YWNjZWw9eGVuIG9yIGJlY2F1c2Ugb3IgdGhlIHhlbi1wbGF0Zm9ybSBkZXZp Y2U/Cj4KPklmIGl0J3MganVzdCBiZWNhdXNlIG9mIGFjY2VsPXhlbiwgd2UgY291bGQgaW50cm9k dWNlIGEKPkFjY2VsQ2xhc3M6Om1heF9jcHVzKCkgbWV0aG9kICh3ZSBhbHNvIGhhdmUgS1ZNLWlt cG9zZWQgQ1BVIGNvdW50Cj5saW1pdHMsIGN1cnJlbnRseSBpbXBsZW1lbnRlZCBpbnNpZGUga3Zt X2luaXQoKSkuCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f XwpYZW4tZGV2ZWwgbWFpbGluZyBsaXN0Clhlbi1kZXZlbEBsaXN0cy54ZW5wcm9qZWN0Lm9yZwpo dHRwczovL2xpc3RzLnhlbnByb2plY3Qub3JnL21haWxtYW4vbGlzdGluZm8veGVuLWRldmVs From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45032) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1evtfh-0000o3-VQ for qemu-devel@nongnu.org; Tue, 13 Mar 2018 19:49:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1evtfe-0000B8-2G for qemu-devel@nongnu.org; Tue, 13 Mar 2018 19:49:34 -0400 Received: from mail-lf0-x241.google.com ([2a00:1450:4010:c07::241]:44242) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1evtfd-0000AU-My for qemu-devel@nongnu.org; Tue, 13 Mar 2018 19:49:29 -0400 Received: by mail-lf0-x241.google.com with SMTP id v9-v6so2015463lfa.11 for ; Tue, 13 Mar 2018 16:49:29 -0700 (PDT) Date: Wed, 14 Mar 2018 09:49:19 +1000 From: Alexey G Message-ID: <20180314094919.00004965@gmail.com> In-Reply-To: <20180312214402.GY3417@localhost.localdomain> References: <4a65e8b30fe9d2a6c1c53b85ef2697f02e01d13f.1520867956.git.x1917x@gmail.com> <20180312194406.GX3417@localhost.localdomain> <20180313065637.00005cee@gmail.com> <20180312214402.GY3417@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH 16/30] q35/xen: Add Xen platform device support for Q35 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: xen-devel@lists.xenproject.org, qemu-devel@nongnu.org, Marcel Apfelbaum , Paolo Bonzini , Richard Henderson , "Michael S. Tsirkin" On Mon, 12 Mar 2018 18:44:02 -0300 Eduardo Habkost wrote: >On Tue, Mar 13, 2018 at 06:56:37AM +1000, Alexey G wrote: >> On Mon, 12 Mar 2018 16:44:06 -0300 >> Eduardo Habkost wrote: >> >> >On Tue, Mar 13, 2018 at 04:34:01AM +1000, Alexey Gerasimenko >> >wrote: >> >> Current Xen/QEMU method to control Xen Platform device on i440 is >> >> a bit odd -- enabling/disabling Xen platform device actually >> >> modifies the QEMU emulated machine type, namely xenfv <--> pc. >> >> >> >> In order to avoid multiplying machine types, use a new way to >> >> control Xen Platform device for QEMU -- "xen-platform-dev" machine >> >> property (bool). To maintain backward compatibility with existing >> >> Xen/QEMU setups, this is only applicable to q35 machine currently. >> >> i440 emulation still uses the old method (i.e. xenfv/pc machine >> >> selection) to control Xen Platform device, this may be changed >> >> later to xen-platform-dev property as well. >> >> >> >> This way we can use a single machine type (q35) and change just >> >> xen-platform-dev value to on/off to control Xen platform device. >> >> >> >> Signed-off-by: Alexey Gerasimenko >> >> --- >> >[...] >> >> diff --git a/qemu-options.hx b/qemu-options.hx >> >> index 6585058c6c..cee0b92028 100644 >> >> --- a/qemu-options.hx >> >> +++ b/qemu-options.hx >> >> @@ -38,6 +38,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ >> >> " dump-guest-core=on|off include guest memory >> >> in a core dump (default=on)\n" " mem-merge=on|off >> >> controls memory merge support (default: on)\n" " >> >> igd-passthru=on|off controls IGD GFX passthrough support >> >> (default=off)\n" >> >> + " xen-platform-dev=on|off controls Xen >> >> Platform device (default=off)\n" " >> >> aes-key-wrap=on|off controls support for AES key wrapping >> >> (default=on)\n" " dea-key-wrap=on|off controls >> >> support for DEA key wrapping (default=on)\n" " >> >> suppress-vmdesc=on|off disables self-describing migration >> >> (default=off)\n" >> > >> >What are the obstacles preventing "-device xen-platform" from >> >working? It would be better than adding a new boolean option to >> >-machine. >> >> I guess the initial assumption was that changing the >> xen_platform_device value in Xen's options may cause some additional >> changes in platform configuration besides adding (or not) the Xen >> Platform device, hence a completely different machine type was chosen >> (xenfv). >> >> At the moment pc,accel=xen/xenfv selection mostly governs >> only the Xen Platform device presence. Also setting max_cpus to >> HVM_MAX_VCPUS depends on it, but this doesn't applicable to a >> 'pc,accel=xen' machine for some reason. >> >> If applying HVM_MAX_VCPUS to max_cpus is really necessary I think >> it's better to set it unconditionally for all 'accel=xen' HVM machine >> types inside xen_enabled() block. Right now it's missing for >> pc,accel=xen and q35,accel=xen. > >If you are talking about MachineClass::max_cpus, note that it is >returned by query-machines, so it's supposed to be a static >value. Changing it a runtime would mean the query-machines value >is incorrect. > >Is HVM_MAX_CPUS higher or lower than 255? If it's higher, does >it mean the current value on pc and q35 isn't accurate? HVM_MAX_VCPUS is 128 currently, but there is an ongoing work from Intel to support more vcpus and >8bit APIC IDs, so this number will likely change soon. According to the code, using HVM_MAX_VCPUS in QEMU is a bit excessive as the maximum number of vcpus is controlled on Xen side anyway. Currently HVM_MAX_VCPUS is used in a one-time check for the maxcpus value (which itself comes from libxl). I think for future compatibility it's better to set mc->max_cpus to HVM_MAX_VCPUS for all accel=xen HVM-supported machine types, not just xenfv. The '-device' approach you suggested seems more preferable than a machine bool property, I'll try switching to it. >Is HVM_MAX_CPUS something that needs to be enabled because of >accel=xen or because or the xen-platform device? > >If it's just because of accel=xen, we could introduce a >AccelClass::max_cpus() method (we also have KVM-imposed CPU count >limits, currently implemented inside kvm_init()).