Re: [Qemu-arm] [PATCH v17 01/10] hw/arm/virt: Add RAS platform version for migration

[Igor Mammedov]

On Mon, 24 Jun 2019 20:19:12 +0800
gengdongjiu <address@hidden> wrote:

> On 2019/6/20 20:04, Igor Mammedov wrote:
> > On Tue, 14 May 2019 04:18:14 -0700
> > Dongjiu Geng <address@hidden> wrote:
> >   
> >> Support this feature since version 4.1, disable it by
> >> default in the old version.
> >>
> >> Signed-off-by: Dongjiu Geng <address@hidden>
> >> ---
> >>  hw/arm/virt.c         | 6 ++++++
> >>  include/hw/arm/virt.h | 1 +
> >>  2 files changed, 7 insertions(+)
> >>
> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> >> index 5331ab7..7bdd41b 100644
> >> --- a/hw/arm/virt.c
> >> +++ b/hw/arm/virt.c
> >> @@ -2043,8 +2043,14 @@ DEFINE_VIRT_MACHINE_AS_LATEST(4, 1)
> >>  
> >>  static void virt_machine_4_0_options(MachineClass *mc)
> >>  {
> >> +    VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
> >> +
> >>      virt_machine_4_1_options(mc);
> >>      compat_props_add(mc->compat_props, hw_compat_4_0, hw_compat_4_0_len);
> >> +    /* Disable memory recovery feature for 4.0 as RAS support was
> >> +     * introduced with 4.1.
> >> +     */
> >> +    vmc->no_ras = true;  
> > 
> > So it would mean that the feature is enabled unconditionally for
> > new machine types and consumes resources whether user needs it or not.
> > 
> > In light of the race for leaner QEMU and faster startup times,
> > it might be better to make RAS optional and make user explicitly
> > enable it using a machine option.  
> 
> I will add a machine option to make RAS optional, do you think we should 
> enable or disable it by default? I think it is better if we enable it by 
> default.
I'd start with disabled by default as it's easy to make it
enabled by default later and we can't do so other way around.

 
> > 
> >   
> >>  }
> >>  DEFINE_VIRT_MACHINE(4, 0)
> >>  
> >> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> >> index 4240709..7f1a033 100644
> >> --- a/include/hw/arm/virt.h
> >> +++ b/include/hw/arm/virt.h
> >> @@ -104,6 +104,7 @@ typedef struct {
> >>      bool disallow_affinity_adjustment;
> >>      bool no_its;
> >>      bool no_pmu;
> >> +    bool no_ras;
> >>      bool claim_edge_triggered_timers;
> >>      bool smbios_old_sys_ver;
> >>      bool no_highmem_ecam;  
> > 
> > .
> >   
>