[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] pc/fwcfg: unbreak migration from qemu-2.5 and q
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH] pc/fwcfg: unbreak migration from qemu-2.5 and qemu-2.6 during firmware boot |
Date: |
Mon, 24 Apr 2017 21:56:04 +0200 |
On Mon, 24 Apr 2017 16:37:31 -0300
Eduardo Habkost <address@hidden> wrote:
> On Mon, Apr 24, 2017 at 09:32:33PM +0200, Igor Mammedov wrote:
> > On Mon, 24 Apr 2017 16:13:17 -0300
> > Eduardo Habkost <address@hidden> wrote:
> >
> > > On Mon, Apr 24, 2017 at 08:58:17PM +0200, Igor Mammedov wrote:
> > > [...]
> > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > > > index f3b372a18f..3f2d96da64 100644
> > > > --- a/hw/i386/pc.c
> > > > +++ b/hw/i386/pc.c
> > > > @@ -1047,7 +1047,7 @@ static void load_linux(PCMachineState *pcms,
> > > > fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_SIZE, setup_size);
> > > > fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA, setup, setup_size);
> > > >
> > > > - if (fw_cfg_dma_enabled(fw_cfg)) {
> > > > + if (!pcmc->linuxboot_dma_disabled && fw_cfg_dma_enabled(fw_cfg)) {
> > >
> > > Why not name the flag just "linuxboot_dma", set it to true by
> > > default at pc_machine_class_init(), and avoid the double
> > > negative?
> > to avoid setting it to true somewhere else, so less thing could go wrong
> > but is you prefer *_enable variant I can switch to it.
>
> I would prefer to. We already have other compat flags initialized
> inside pc_machine_class_init(), so this would fit nicely there.
how about 'use_linuxboot_mmio' instead, it will remove negation
and let me not to touch pc_machine_class_init()?
>
> >
> > >
> > > [...]
> > > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > > > index f278b3ae89..ff6f13b61b 100644
> > > > --- a/include/hw/i386/pc.h
> > > > +++ b/include/hw/i386/pc.h
> > > > @@ -151,6 +151,8 @@ struct PCMachineClass {
> > > > bool save_tsc_khz;
> > > > /* generate legacy CPU hotplug AML */
> > > > bool legacy_cpu_hotplug;
> > > > +
> > >
> > > A one-line description of the consequences of setting/clearing
> > > the flag would be nice.
> > will fix in v2
>
> Thanks!
>
> >
> > >
> > > > + bool linuxboot_dma_disabled;
> > > > };
> > > >
> > > > #define TYPE_PC_MACHINE "generic-pc-machine"
> > > > @@ -432,10 +434,6 @@ bool e820_get_entry(int, uint32_t, uint64_t *,
> > > > uint64_t *);
> > > > #define PC_COMPAT_2_6 \
> > > > HW_COMPAT_2_6 \
> > > > {\
> > > > - .driver = "fw_cfg_io",\
> > > > - .property = "dma_enabled",\
> > > > - .value = "off",\
> > > > - },{\
> > > > .driver = TYPE_X86_CPU,\
> > > > .property = "cpuid-0xb",\
> > > > .value = "off",\
> > > > --
> > > > 2.11.0 (Apple Git-81)
> > > >
> > >
> >
>