From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laszlo Ersek Subject: Re: [Qemu-devel] [PATCH 3/4] pc: Add 2.9 machine-types Date: Wed, 4 Jan 2017 15:01:07 +0100 Message-ID: References: <1482866480-26208-1-git-send-email-ehabkost@redhat.com> <1482866480-26208-4-git-send-email-ehabkost@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: Haozhong Zhang , kvm@vger.kernel.org, "Michael S. Tsirkin" , Marcelo Tosatti , Igor Mammedov , Paolo Bonzini To: Eduardo Habkost , qemu-devel@nongnu.org Return-path: Received: from mx1.redhat.com ([209.132.183.28]:56278 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760646AbdADOBQ (ORCPT ); Wed, 4 Jan 2017 09:01:16 -0500 In-Reply-To: <1482866480-26208-4-git-send-email-ehabkost@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 12/27/16 20:21, Eduardo Habkost wrote: > Cc: "Michael S. Tsirkin" > Cc: Laszlo Ersek > Cc: Igor Mammedov > Signed-off-by: Eduardo Habkost > --- > include/hw/i386/pc.h | 1 + > hw/i386/pc_piix.c | 15 ++++++++++++--- > hw/i386/pc_q35.c | 13 +++++++++++-- > 3 files changed, 24 insertions(+), 5 deletions(-) > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index b22e699..ceeacca 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -375,6 +375,7 @@ int e820_get_num_entries(void); > bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); > > #define PC_COMPAT_2_8 \ > + HW_COMPAT_2_8 > > #define PC_COMPAT_2_7 \ > HW_COMPAT_2_7 \ Here I would recommend two things: - introduce an empty PC_COMPAT_2_9 macro - in the PC_COMPAT_2_8 macro being modified, add a trailing backslash after HW_COMPAT_2_8 Both of these aim at keeping up the current pattern; namely, for the most recent machine type, a PC_COMPAT macro should exist (even if empty / unused), plus each PC_COMPAT macro should be extensible by plain appending (hence the final trailing backslash). Functionally the above hunk is good of course, so I'll leave it to you whether you actually want to address my comments. The rest looks fine; I should be able to rebase my broadcast SMI (v5) stuff to this. Changed or unchanged: Reviewed-by: Laszlo Ersek Thanks Laszlo > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index 5e1adbe..9f102aa 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -437,13 +437,24 @@ static void pc_i440fx_machine_options(MachineClass *m) > m->default_display = "std"; > } > > -static void pc_i440fx_2_8_machine_options(MachineClass *m) > +static void pc_i440fx_2_9_machine_options(MachineClass *m) > { > pc_i440fx_machine_options(m); > m->alias = "pc"; > m->is_default = 1; > } > > +DEFINE_I440FX_MACHINE(v2_9, "pc-i440fx-2.9", NULL, > + pc_i440fx_2_9_machine_options); > + > +static void pc_i440fx_2_8_machine_options(MachineClass *m) > +{ > + pc_i440fx_2_9_machine_options(m); > + m->is_default = 0; > + m->alias = NULL; > + SET_MACHINE_COMPAT(m, PC_COMPAT_2_8); > +} > + > DEFINE_I440FX_MACHINE(v2_8, "pc-i440fx-2.8", NULL, > pc_i440fx_2_8_machine_options); > > @@ -451,8 +462,6 @@ DEFINE_I440FX_MACHINE(v2_8, "pc-i440fx-2.8", NULL, > static void pc_i440fx_2_7_machine_options(MachineClass *m) > { > pc_i440fx_2_8_machine_options(m); > - m->is_default = 0; > - m->alias = NULL; > SET_MACHINE_COMPAT(m, PC_COMPAT_2_7); > } > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index d042fe0..dd792a8 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -301,19 +301,28 @@ static void pc_q35_machine_options(MachineClass *m) > m->max_cpus = 288; > } > > -static void pc_q35_2_8_machine_options(MachineClass *m) > +static void pc_q35_2_9_machine_options(MachineClass *m) > { > pc_q35_machine_options(m); > m->alias = "q35"; > } > > +DEFINE_Q35_MACHINE(v2_9, "pc-q35-2.9", NULL, > + pc_q35_2_9_machine_options); > + > +static void pc_q35_2_8_machine_options(MachineClass *m) > +{ > + pc_q35_2_9_machine_options(m); > + m->alias = NULL; > + SET_MACHINE_COMPAT(m, PC_COMPAT_2_8); > +} > + > DEFINE_Q35_MACHINE(v2_8, "pc-q35-2.8", NULL, > pc_q35_2_8_machine_options); > > static void pc_q35_2_7_machine_options(MachineClass *m) > { > pc_q35_2_8_machine_options(m); > - m->alias = NULL; > m->max_cpus = 255; > SET_MACHINE_COMPAT(m, PC_COMPAT_2_7); > } >