From mboxrd@z Thu Jan 1 00:00:00 1970 Received: by 2002:a5d:6089:0:0:0:0:0 with SMTP id w9csp510499wrt; Thu, 22 Nov 2018 02:39:49 -0800 (PST) X-Google-Smtp-Source: AFSGD/UOhxsJNzszHgk591pqReH6xJkq1u4V93PoOQlPuYfpj0kZhWrPQVafWrJgzcdWMmxX0oa6 X-Received: by 2002:a25:c008:: with SMTP id c8-v6mr6091167ybf.394.1542883189478; Thu, 22 Nov 2018 02:39:49 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1542883189; cv=none; d=google.com; s=arc-20160816; b=UdnwlB2q1oHaHNY/6EknVwDixB3rCwnynzl6NL6YOjKA/dEGvFZsk0X158IzCNgOZY /qZBTW4o4tG7jt7I/trvKEm2+bX9kMle3wXY7ZK0iZDQGZhXSkFkooF0TIDtf0QFMDV0 1mUxtV2nGF03chFLParzLtEq0hQ4ZZ/SduGXxbUAVhBv76AgTksaiC5HfseVPL0Ik+pL w/3bMMeVs6SJhLCyCQNmqYQnI0/Xcpkp7GPYHBFz/eX8lV4c9XL8S6HNCYUhFus08x9+ ypkPiBAQCZJ+BfjNmjBftb4j3tUduZi0V2OOe9wb4Cxosyx1U+pjMzovWVerLNGXQeYz UBKw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:subject :content-transfer-encoding:mime-version:references:in-reply-to :message-id:to:from:date; bh=fUOJAXGe0kD0gyfGncEfhIitUs5fuxHWL1VCBAO6Aew=; b=qbnad3LvtsxNh1ZRJr2H9oH52jjOqTyCWYiE4xmfNS03xeTHM/D0bQUhqmXfMbKbaS 15scU5/I1mzGnXnzvK2oIow6hhApcdLngAV0CqY2d/BlClDuHJZVntXcBLO9P5E+jrd8 YWKtTQ0CVB8MlPwYyNdIWOzItsnb3IdN2WcydroxDZDx0ZVqxNNowRVGb/H5i6/DZKSA 2G3pDt5F8KIQXhlyvRNEje+gVDZEJCbNjEnIApmPs3Py5lsc7IqsUKbjdI5eadFD4D8I gKRhWekjbcMWlZ8u+ugD0G/03ySdqXyjAweqh+8TF0k8jx+DpgPui4ZGWg3+P/Vi+nao xZcw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom="qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id q6-v6si32914813ybd.31.2018.11.22.02.39.49 for (version=TLS1 cipher=AES128-SHA bits=128/128); Thu, 22 Nov 2018 02:39:49 -0800 (PST) Received-SPF: pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) client-ip=2001:4830:134:3::11; Authentication-Results: mx.google.com; spf=pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom="qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from localhost ([::1]:45077 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gPmOi-0005wq-Qr for alex.bennee@linaro.org; Thu, 22 Nov 2018 05:39:48 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43423) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gPmOC-0005e0-84 for qemu-arm@nongnu.org; Thu, 22 Nov 2018 05:39:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gPmO8-0006OA-Ve for qemu-arm@nongnu.org; Thu, 22 Nov 2018 05:39:16 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58032) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gPmO8-0006L7-Nf; Thu, 22 Nov 2018 05:39:12 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7974C3001C33; Thu, 22 Nov 2018 10:39:10 +0000 (UTC) Received: from localhost (unknown [10.43.2.182]) by smtp.corp.redhat.com (Postfix) with ESMTP id 390F175719; Thu, 22 Nov 2018 10:39:05 +0000 (UTC) Date: Thu, 22 Nov 2018 11:39:03 +0100 From: Igor Mammedov To: Samuel Ortiz Message-ID: <20181122113903.46e3fefd@redhat.com> In-Reply-To: <20181121143816.GD4426@caravaggio> References: <20181105014047.26447-1-sameo@linux.intel.com> <20181116172919.43f3e27d@redhat.com> <20181119163110.2f357f40@redhat.com> <20181121072954-mutt-send-email-mst@kernel.org> <20181121151526.5785b43f@redhat.com> <20181121143816.GD4426@caravaggio> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.47]); Thu, 22 Nov 2018 10:39:10 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: Re: [Qemu-arm] [Qemu-devel] [PATCH v5 00/24] ACPI reorganization for hardware-reduced API addition X-BeenThere: qemu-arm@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Maydell , Stefano Stabellini , Eduardo Habkost , "Michael S. Tsirkin" , qemu-devel@nongnu.org, Shannon Zhao , qemu-arm@nongnu.org, Paolo Bonzini , Anthony Perard , xen-devel@lists.xenproject.org, Richard Henderson Errors-To: qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org Sender: "Qemu-arm" X-TUID: UH4OyZTJE+j+ On Wed, 21 Nov 2018 15:38:16 +0100 Samuel Ortiz wrote: > Igor, > > On Wed, Nov 21, 2018 at 03:15:26PM +0100, Igor Mammedov wrote: > > On Wed, 21 Nov 2018 07:35:47 -0500 > > "Michael S. Tsirkin" wrote: > > > > > On Mon, Nov 19, 2018 at 04:31:10PM +0100, Igor Mammedov wrote: > > > > On Fri, 16 Nov 2018 17:37:54 +0100 > > > > Paolo Bonzini wrote: > > > > > > > > > On 16/11/18 17:29, Igor Mammedov wrote: > > > > > > General suggestions for this series: > > > > > > 1. Preferably don't do multiple changes within a patch > > > > > > neither post huge patches (unless it's pure code movement). > > > > > > (it's easy to squash patches later it necessary) > > > > > > 2. Start small, pick a table generalize it and send as > > > > > > one small patchset. Tables are often independent > > > > > > and it's much easier on both author/reviewer to agree upon > > > > > > changes and rewrite it if necessary. > > > > > > > > > > How would that be done? This series is on the bigger side, agreed, but > > > > > most of it is really just code movement. It's a starting point, having > > > > > a generic ACPI library is way beyond what this is trying to do. > > > > I've tried to give suggestions how to restructure series > > > > on per patch basis. In my opinion it quite possible to split > > > > series in several smaller ones and it should really help with > > > > making series cleaner and easier/faster to review/amend/merge > > > > vs what we have in v5. > > > > (it's more frustrating to rework large series vs smaller one) > > > > > > > > If something isn't clear, it's easy to reach out to me here > > > > or directly (email/irc/github) for clarification/feed back. > > > > > > I assume the #1 goal is to add reduced HW support. So another > > > option to speed up merging is to just go ahead and duplicate a > > > bunch of code e.g. in pc_virt.c acpi/reduced.c or in any other > > > file. > > > This way it might be easier to see what's common code and what isn't. > > > And I think offline Igor said he might prefer that way. Right Igor? > > You mean probably 'x86 reduced hw' support. That's was what I've > > already suggested for PCI AML code during patch review. Just don't > > call it generic when it's not and place code in hw/i386/ directory beside > > acpi-build.c. It might apply to some other tables (i.e. complex cases). > > > > On per patch review I gave suggestions how to amend series to make > > it acceptable without doing complex refactoring and pointed out > > places we probably shouldn't refactor now and just duplicate as > > it's too complex or not clear how to generalize it yet. > > > > Problem with duplication is that a random contributor is not > > around to clean code up after a feature is merged and we end up > > with a bunch of messy code. > > > > A word to the contributors, > > Don't do refactoring in silence, keep discussing approaches here, > > suggest alternatives. That way it's easier to reach a compromise > > and merge it with less iterations. And if you do split it in smaller > > parts, the process should go even faster. > > > > I'll sent a small RSDP refactoring series for reference. > I was already working on the RSDP changes. Let me know if I should drop > that work too. Go ahead, you can reuse RSDP fixes I've just posted (you are CCed) if you haven't written them yet on your own. > Cheers, > Samuel. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Igor Mammedov Subject: Re: [Qemu-devel] [PATCH v5 00/24] ACPI reorganization for hardware-reduced API addition Date: Thu, 22 Nov 2018 11:39:03 +0100 Message-ID: <20181122113903.46e3fefd@redhat.com> References: <20181105014047.26447-1-sameo@linux.intel.com> <20181116172919.43f3e27d@redhat.com> <20181119163110.2f357f40@redhat.com> <20181121072954-mutt-send-email-mst@kernel.org> <20181121151526.5785b43f@redhat.com> <20181121143816.GD4426@caravaggio> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1gPmO8-0008Ly-8B for xen-devel@lists.xenproject.org; Thu, 22 Nov 2018 10:39:12 +0000 In-Reply-To: <20181121143816.GD4426@caravaggio> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" To: Samuel Ortiz Cc: Peter Maydell , Stefano Stabellini , Eduardo Habkost , "Michael S. Tsirkin" , qemu-devel@nongnu.org, Shannon Zhao , qemu-arm@nongnu.org, Paolo Bonzini , Anthony Perard , xen-devel@lists.xenproject.org, Richard Henderson List-Id: xen-devel@lists.xenproject.org T24gV2VkLCAyMSBOb3YgMjAxOCAxNTozODoxNiArMDEwMApTYW11ZWwgT3J0aXogPHNhbWVvQGxp bnV4LmludGVsLmNvbT4gd3JvdGU6Cgo+IElnb3IsCj4gCj4gT24gV2VkLCBOb3YgMjEsIDIwMTgg YXQgMDM6MTU6MjZQTSArMDEwMCwgSWdvciBNYW1tZWRvdiB3cm90ZToKPiA+IE9uIFdlZCwgMjEg Tm92IDIwMTggMDc6MzU6NDcgLTA1MDAKPiA+ICJNaWNoYWVsIFMuIFRzaXJraW4iIDxtc3RAcmVk aGF0LmNvbT4gd3JvdGU6Cj4gPiAgIAo+ID4gPiBPbiBNb24sIE5vdiAxOSwgMjAxOCBhdCAwNDoz MToxMFBNICswMTAwLCBJZ29yIE1hbW1lZG92IHdyb3RlOiAgCj4gPiA+ID4gT24gRnJpLCAxNiBO b3YgMjAxOCAxNzozNzo1NCArMDEwMAo+ID4gPiA+IFBhb2xvIEJvbnppbmkgPHBib256aW5pQHJl ZGhhdC5jb20+IHdyb3RlOgo+ID4gPiA+ICAgICAKPiA+ID4gPiA+IE9uIDE2LzExLzE4IDE3OjI5 LCBJZ29yIE1hbW1lZG92IHdyb3RlOiAgICAKPiA+ID4gPiA+ID4gR2VuZXJhbCBzdWdnZXN0aW9u cyBmb3IgdGhpcyBzZXJpZXM6Cj4gPiA+ID4gPiA+ICAgMS4gUHJlZmVyYWJseSBkb24ndCBkbyBt dWx0aXBsZSBjaGFuZ2VzIHdpdGhpbiBhIHBhdGNoCj4gPiA+ID4gPiA+ICAgICAgbmVpdGhlciBw b3N0IGh1Z2UgcGF0Y2hlcyAodW5sZXNzIGl0J3MgcHVyZSBjb2RlIG1vdmVtZW50KS4KPiA+ID4g PiA+ID4gICAgICAoaXQncyBlYXN5IHRvIHNxdWFzaCBwYXRjaGVzIGxhdGVyIGl0IG5lY2Vzc2Fy eSkKPiA+ID4gPiA+ID4gICAyLiBTdGFydCBzbWFsbCwgcGljayBhIHRhYmxlIGdlbmVyYWxpemUg aXQgYW5kIHNlbmQgYXMKPiA+ID4gPiA+ID4gICAgICBvbmUgc21hbGwgcGF0Y2hzZXQuIFRhYmxl cyBhcmUgb2Z0ZW4gaW5kZXBlbmRlbnQKPiA+ID4gPiA+ID4gICAgICBhbmQgaXQncyBtdWNoIGVh c2llciBvbiBib3RoIGF1dGhvci9yZXZpZXdlciB0byBhZ3JlZSB1cG9uCj4gPiA+ID4gPiA+ICAg ICAgY2hhbmdlcyBhbmQgcmV3cml0ZSBpdCBpZiBuZWNlc3NhcnkuICAgICAgCj4gPiA+ID4gPiAK PiA+ID4gPiA+IEhvdyB3b3VsZCB0aGF0IGJlIGRvbmU/ICBUaGlzIHNlcmllcyBpcyBvbiB0aGUg YmlnZ2VyIHNpZGUsIGFncmVlZCwgYnV0Cj4gPiA+ID4gPiBtb3N0IG9mIGl0IGlzIHJlYWxseSBq dXN0IGNvZGUgbW92ZW1lbnQuICBJdCdzIGEgc3RhcnRpbmcgcG9pbnQsIGhhdmluZwo+ID4gPiA+ ID4gYSBnZW5lcmljIEFDUEkgbGlicmFyeSBpcyB3YXkgYmV5b25kIHdoYXQgdGhpcyBpcyB0cnlp bmcgdG8gZG8uICAgIAo+ID4gPiA+IEkndmUgdHJpZWQgdG8gZ2l2ZSBzdWdnZXN0aW9ucyBob3cg dG8gcmVzdHJ1Y3R1cmUgc2VyaWVzCj4gPiA+ID4gb24gcGVyIHBhdGNoIGJhc2lzLiBJbiBteSBv cGluaW9uIGl0IHF1aXRlIHBvc3NpYmxlIHRvIHNwbGl0Cj4gPiA+ID4gc2VyaWVzIGluIHNldmVy YWwgc21hbGxlciBvbmVzIGFuZCBpdCBzaG91bGQgcmVhbGx5IGhlbHAgd2l0aAo+ID4gPiA+IG1h a2luZyBzZXJpZXMgY2xlYW5lciBhbmQgZWFzaWVyL2Zhc3RlciB0byByZXZpZXcvYW1lbmQvbWVy Z2UKPiA+ID4gPiB2cyB3aGF0IHdlIGhhdmUgaW4gdjUuCj4gPiA+ID4gKGl0J3MgbW9yZSBmcnVz dHJhdGluZyB0byByZXdvcmsgbGFyZ2Ugc2VyaWVzIHZzIHNtYWxsZXIgb25lKQo+ID4gPiA+IAo+ ID4gPiA+IElmIHNvbWV0aGluZyBpc24ndCBjbGVhciwgaXQncyBlYXN5IHRvIHJlYWNoIG91dCB0 byBtZSBoZXJlCj4gPiA+ID4gb3IgZGlyZWN0bHkgKGVtYWlsL2lyYy9naXRodWIpIGZvciBjbGFy aWZpY2F0aW9uL2ZlZWQgYmFjay4gICAgCj4gPiA+IAo+ID4gPiBJIGFzc3VtZSB0aGUgIzEgZ29h bCBpcyB0byBhZGQgcmVkdWNlZCBIVyBzdXBwb3J0LiAgU28gYW5vdGhlcgo+ID4gPiBvcHRpb24g dG8gc3BlZWQgdXAgbWVyZ2luZyBpcyB0byBqdXN0IGdvIGFoZWFkIGFuZCBkdXBsaWNhdGUgYQo+ ID4gPiBidW5jaCBvZiBjb2RlIGUuZy4gaW4gcGNfdmlydC5jIGFjcGkvcmVkdWNlZC5jIG9yIGlu IGFueSBvdGhlcgo+ID4gPiBmaWxlLgo+ID4gPiBUaGlzIHdheSBpdCBtaWdodCBiZSBlYXNpZXIg dG8gc2VlIHdoYXQncyBjb21tb24gY29kZSBhbmQgd2hhdCBpc24ndC4KPiA+ID4gQW5kIEkgdGhp bmsgb2ZmbGluZSBJZ29yIHNhaWQgaGUgbWlnaHQgcHJlZmVyIHRoYXQgd2F5LiBSaWdodCBJZ29y PyAgCj4gPiBZb3UgbWVhbiBwcm9iYWJseSAneDg2IHJlZHVjZWQgaHcnIHN1cHBvcnQuIFRoYXQn cyB3YXMgd2hhdCBJJ3ZlCj4gPiBhbHJlYWR5IHN1Z2dlc3RlZCBmb3IgUENJIEFNTCBjb2RlIGR1 cmluZyBwYXRjaCByZXZpZXcuIEp1c3QgZG9uJ3QKPiA+IGNhbGwgaXQgZ2VuZXJpYyB3aGVuIGl0 J3Mgbm90IGFuZCBwbGFjZSBjb2RlIGluIGh3L2kzODYvIGRpcmVjdG9yeSBiZXNpZGUKPiA+IGFj cGktYnVpbGQuYy4gSXQgbWlnaHQgYXBwbHkgdG8gc29tZSBvdGhlciB0YWJsZXMgKGkuZS4gY29t cGxleCBjYXNlcykuCj4gPiAKPiA+IE9uIHBlciBwYXRjaCByZXZpZXcgSSBnYXZlIHN1Z2dlc3Rp b25zIGhvdyB0byBhbWVuZCBzZXJpZXMgdG8gbWFrZQo+ID4gaXQgYWNjZXB0YWJsZSB3aXRob3V0 IGRvaW5nIGNvbXBsZXggcmVmYWN0b3JpbmcgYW5kIHBvaW50ZWQgb3V0Cj4gPiBwbGFjZXMgd2Ug cHJvYmFibHkgc2hvdWxkbid0IHJlZmFjdG9yIG5vdyBhbmQganVzdCBkdXBsaWNhdGUgYXMKPiA+ IGl0J3MgdG9vIGNvbXBsZXggb3Igbm90IGNsZWFyIGhvdyB0byBnZW5lcmFsaXplIGl0IHlldC4K PiA+IAo+ID4gUHJvYmxlbSB3aXRoIGR1cGxpY2F0aW9uIGlzIHRoYXQgYSByYW5kb20gY29udHJp YnV0b3IgaXMgbm90Cj4gPiBhcm91bmQgdG8gY2xlYW4gY29kZSB1cCBhZnRlciBhIGZlYXR1cmUg aXMgbWVyZ2VkIGFuZCB3ZSBlbmQgdXAKPiA+IHdpdGggYSBidW5jaCBvZiBtZXNzeSBjb2RlLgo+ ID4gCj4gPiBBIHdvcmQgdG8gdGhlIGNvbnRyaWJ1dG9ycywKPiA+IERvbid0IGRvIHJlZmFjdG9y aW5nIGluIHNpbGVuY2UsIGtlZXAgZGlzY3Vzc2luZyBhcHByb2FjaGVzIGhlcmUsCj4gPiBzdWdn ZXN0IGFsdGVybmF0aXZlcy4gVGhhdCB3YXkgaXQncyBlYXNpZXIgdG8gcmVhY2ggYSBjb21wcm9t aXNlCj4gPiBhbmQgbWVyZ2UgaXQgd2l0aCBsZXNzIGl0ZXJhdGlvbnMuIEFuZCBpZiB5b3UgZG8g c3BsaXQgaXQgaW4gc21hbGxlcgo+ID4gcGFydHMsIHRoZSBwcm9jZXNzIHNob3VsZCBnbyBldmVu IGZhc3Rlci4KPiA+IAo+ID4gSSdsbCBzZW50IGEgc21hbGwgUlNEUCByZWZhY3RvcmluZyBzZXJp ZXMgZm9yIHJlZmVyZW5jZS4gIAo+IEkgd2FzIGFscmVhZHkgd29ya2luZyBvbiB0aGUgUlNEUCBj aGFuZ2VzLiBMZXQgbWUga25vdyBpZiBJIHNob3VsZCBkcm9wCj4gdGhhdCB3b3JrIHRvby4KR28g YWhlYWQsCnlvdSBjYW4gcmV1c2UgUlNEUCBmaXhlcyBJJ3ZlIGp1c3QgcG9zdGVkICh5b3UgYXJl IENDZWQpCmlmIHlvdSBoYXZlbid0IHdyaXR0ZW4gdGhlbSB5ZXQgb24geW91ciBvd24uCgogCj4g Q2hlZXJzLAo+IFNhbXVlbC4KCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fXwpYZW4tZGV2ZWwgbWFpbGluZyBsaXN0Clhlbi1kZXZlbEBsaXN0cy54ZW5wcm9q ZWN0Lm9yZwpodHRwczovL2xpc3RzLnhlbnByb2plY3Qub3JnL21haWxtYW4vbGlzdGluZm8veGVu LWRldmVs From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43453) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gPmOE-0005fo-Kn for qemu-devel@nongnu.org; Thu, 22 Nov 2018 05:39:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gPmOD-0006df-L8 for qemu-devel@nongnu.org; Thu, 22 Nov 2018 05:39:18 -0500 Date: Thu, 22 Nov 2018 11:39:03 +0100 From: Igor Mammedov Message-ID: <20181122113903.46e3fefd@redhat.com> In-Reply-To: <20181121143816.GD4426@caravaggio> References: <20181105014047.26447-1-sameo@linux.intel.com> <20181116172919.43f3e27d@redhat.com> <20181119163110.2f357f40@redhat.com> <20181121072954-mutt-send-email-mst@kernel.org> <20181121151526.5785b43f@redhat.com> <20181121143816.GD4426@caravaggio> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 00/24] ACPI reorganization for hardware-reduced API addition List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Samuel Ortiz Cc: "Michael S. Tsirkin" , Peter Maydell , Stefano Stabellini , qemu-devel@nongnu.org, Shannon Zhao , qemu-arm@nongnu.org, xen-devel@lists.xenproject.org, Anthony Perard , Paolo Bonzini , Richard Henderson , Eduardo Habkost On Wed, 21 Nov 2018 15:38:16 +0100 Samuel Ortiz wrote: > Igor, > > On Wed, Nov 21, 2018 at 03:15:26PM +0100, Igor Mammedov wrote: > > On Wed, 21 Nov 2018 07:35:47 -0500 > > "Michael S. Tsirkin" wrote: > > > > > On Mon, Nov 19, 2018 at 04:31:10PM +0100, Igor Mammedov wrote: > > > > On Fri, 16 Nov 2018 17:37:54 +0100 > > > > Paolo Bonzini wrote: > > > > > > > > > On 16/11/18 17:29, Igor Mammedov wrote: > > > > > > General suggestions for this series: > > > > > > 1. Preferably don't do multiple changes within a patch > > > > > > neither post huge patches (unless it's pure code movement). > > > > > > (it's easy to squash patches later it necessary) > > > > > > 2. Start small, pick a table generalize it and send as > > > > > > one small patchset. Tables are often independent > > > > > > and it's much easier on both author/reviewer to agree upon > > > > > > changes and rewrite it if necessary. > > > > > > > > > > How would that be done? This series is on the bigger side, agreed, but > > > > > most of it is really just code movement. It's a starting point, having > > > > > a generic ACPI library is way beyond what this is trying to do. > > > > I've tried to give suggestions how to restructure series > > > > on per patch basis. In my opinion it quite possible to split > > > > series in several smaller ones and it should really help with > > > > making series cleaner and easier/faster to review/amend/merge > > > > vs what we have in v5. > > > > (it's more frustrating to rework large series vs smaller one) > > > > > > > > If something isn't clear, it's easy to reach out to me here > > > > or directly (email/irc/github) for clarification/feed back. > > > > > > I assume the #1 goal is to add reduced HW support. So another > > > option to speed up merging is to just go ahead and duplicate a > > > bunch of code e.g. in pc_virt.c acpi/reduced.c or in any other > > > file. > > > This way it might be easier to see what's common code and what isn't. > > > And I think offline Igor said he might prefer that way. Right Igor? > > You mean probably 'x86 reduced hw' support. That's was what I've > > already suggested for PCI AML code during patch review. Just don't > > call it generic when it's not and place code in hw/i386/ directory beside > > acpi-build.c. It might apply to some other tables (i.e. complex cases). > > > > On per patch review I gave suggestions how to amend series to make > > it acceptable without doing complex refactoring and pointed out > > places we probably shouldn't refactor now and just duplicate as > > it's too complex or not clear how to generalize it yet. > > > > Problem with duplication is that a random contributor is not > > around to clean code up after a feature is merged and we end up > > with a bunch of messy code. > > > > A word to the contributors, > > Don't do refactoring in silence, keep discussing approaches here, > > suggest alternatives. That way it's easier to reach a compromise > > and merge it with less iterations. And if you do split it in smaller > > parts, the process should go even faster. > > > > I'll sent a small RSDP refactoring series for reference. > I was already working on the RSDP changes. Let me know if I should drop > that work too. Go ahead, you can reuse RSDP fixes I've just posted (you are CCed) if you haven't written them yet on your own. > Cheers, > Samuel.