Re: [Qemu-devel] [PATCH for-1.6] pc: fix up pc initialization

[Anthony Liguori <aliguori@us.ibm.com>]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> commit 7f3e341a008c585deed174eaf1f826c88c67948a
>     hw/misc: don't create pvpanic device by default
> was mismerged: as a result, pvpanic is enabled in 1.6
>
> Fix this up, clean up a trivial code duplication
> and add a comment explaining why we special-case 1.5
> with respect to pvpanic.
>
> Reported-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Thanks for catching this.  I'm a little disturbed by this.  I use git-am
--3way specifically to avoid problems from fuzzing but I guess merge
artifacts are possible.

> ---
>  hw/i386/pc_piix.c | 6 +++---
>  hw/i386/pc_q35.c  | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 55c24f2..c58f0f4 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -252,12 +252,12 @@ static void pc_init_pci(QEMUMachineInitArgs *args)
>  static void pc_init_pci_1_6(QEMUMachineInitArgs *args)
>  {
>      has_pci_info = false;
> -    has_pvpanic = true;
>      pc_init_pci(args);
>  }
>  
>  static void pc_init_pci_1_5(QEMUMachineInitArgs *args)
>  {
> +    has_pvpanic = true;
>      pc_init_pci_1_6(args);
>  }

I'd prefer we stick to the minimal fix.  If you want to refactor the
code lets do it separately.  I just sent a patch fixing the merge
problem.

Regards,

Anthony Liguori

>  
> @@ -265,8 +265,8 @@ static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
>  {
>      x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
>      x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, CPUID_EXT_PCLMULQDQ);
> -    has_pci_info = false;
> -    pc_init_pci(args);
> +    /* 1.5 was special as it has pvpanic as a builtin */
> +    pc_init_pci_1_6(args);
>  }
>  
>  static void pc_init_pci_1_3(QEMUMachineInitArgs *args)
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index bd25071..968b22b 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -221,12 +221,12 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
>  static void pc_q35_init_1_6(QEMUMachineInitArgs *args)
>  {
>      has_pci_info = false;
> -    has_pvpanic = true;
>      pc_q35_init(args);
>  }
>  
>  static void pc_q35_init_1_5(QEMUMachineInitArgs *args)
>  {
> +    has_pvpanic = true;
>      pc_q35_init_1_6(args);
>  }
>  
> @@ -234,8 +234,8 @@ static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
>  {
>      x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
>      x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, CPUID_EXT_PCLMULQDQ);
> -    has_pci_info = false;
> -    pc_q35_init(args);
> +    /* 1.5 was special as it has pvpanic as a builtin */
> +    pc_q35_init_1_6(args);
>  }
>  
>  static QEMUMachine pc_q35_machine_v1_6 = {
> -- 
> MST