[Qemu-devel] [PATCH v18 1/6] hw/arm/virt: Introduce RAS platform version and RAS machine option

Xiang Zheng posted 6 patches 6 years, 5 months ago
Maintainers: Eduardo Habkost <ehabkost@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Shannon Zhao <shannon.zhaosl@gmail.com>, Peter Maydell <peter.maydell@linaro.org>, Marcelo Tosatti <mtosatti@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <rth@twiddle.net>, Igor Mammedov <imammedo@redhat.com>
There is a newer version of this series
[Qemu-devel] [PATCH v18 1/6] hw/arm/virt: Introduce RAS platform version and RAS machine option
Posted by Xiang Zheng 6 years, 5 months ago
From: Dongjiu Geng <gengdongjiu@huawei.com>

Support RAS Virtualization feature since version 4.2, disable it by
default in the old versions. Also add a machine option which allows user
to enable it explicitly.

Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
Signed-off-by: Xiang Zheng <zhengxiang9@huawei.com>
---
 hw/arm/virt.c         | 33 +++++++++++++++++++++++++++++++++
 include/hw/arm/virt.h |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index d74538b021..e0451433c8 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1783,6 +1783,20 @@ static void virt_set_its(Object *obj, bool value, Error **errp)
     vms->its = value;
 }
 
+static bool virt_get_ras(Object *obj, Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    return vms->ras;
+}
+
+static void virt_set_ras(Object *obj, bool value, Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    vms->ras = value;
+}
+
 static char *virt_get_gic_version(Object *obj, Error **errp)
 {
     VirtMachineState *vms = VIRT_MACHINE(obj);
@@ -2026,6 +2040,19 @@ static void virt_instance_init(Object *obj)
                                     "Valid values are none and smmuv3",
                                     NULL);
 
+    if (vmc->no_ras) {
+        vms->ras = false;
+    } else {
+        /* Default disallows RAS instantiation */
+        vms->ras = false;
+        object_property_add_bool(obj, "ras", virt_get_ras,
+                                 virt_set_ras, NULL);
+        object_property_set_description(obj, "ras",
+                                        "Set on/off to enable/disable "
+                                        "RAS instantiation",
+                                        NULL);
+    }
+
     vms->irqmap = a15irqmap;
 
     virt_flash_create(vms);
@@ -2058,8 +2085,14 @@ DEFINE_VIRT_MACHINE_AS_LATEST(4, 2)
 
 static void virt_machine_4_1_options(MachineClass *mc)
 {
+    VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
+
     virt_machine_4_2_options(mc);
     compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
+    /* Disable memory recovery feature for 4.1 as RAS support was
+     * introduced with 4.2.
+     */
+    vmc->no_ras = true;
 }
 DEFINE_VIRT_MACHINE(4, 1)
 
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index a72094204e..04ab42ca42 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -103,6 +103,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;
@@ -119,6 +120,7 @@ typedef struct {
     bool highmem_ecam;
     bool its;
     bool virt;
+    bool ras;
     int32_t gic_version;
     VirtIOMMUType iommu;
     struct arm_boot_info bootinfo;
-- 
2.19.1



Re: [PATCH v18 1/6] hw/arm/virt: Introduce RAS platform version and RAS machine option
Posted by Peter Maydell 6 years, 4 months ago
On Fri, 6 Sep 2019 at 09:33, Xiang Zheng <zhengxiang9@huawei.com> wrote:
>
> From: Dongjiu Geng <gengdongjiu@huawei.com>
>
> Support RAS Virtualization feature since version 4.2, disable it by
> default in the old versions. Also add a machine option which allows user
> to enable it explicitly.
>
> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> Signed-off-by: Xiang Zheng <zhengxiang9@huawei.com>
> ---
>  hw/arm/virt.c         | 33 +++++++++++++++++++++++++++++++++
>  include/hw/arm/virt.h |  2 ++
>  2 files changed, 35 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index d74538b021..e0451433c8 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1783,6 +1783,20 @@ static void virt_set_its(Object *obj, bool value, Error **errp)
>      vms->its = value;
>  }
>
> +static bool virt_get_ras(Object *obj, Error **errp)
> +{
> +    VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +    return vms->ras;
> +}
> +
> +static void virt_set_ras(Object *obj, bool value, Error **errp)
> +{
> +    VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +    vms->ras = value;
> +}
> +
>  static char *virt_get_gic_version(Object *obj, Error **errp)
>  {
>      VirtMachineState *vms = VIRT_MACHINE(obj);
> @@ -2026,6 +2040,19 @@ static void virt_instance_init(Object *obj)
>                                      "Valid values are none and smmuv3",
>                                      NULL);
>
> +    if (vmc->no_ras) {
> +        vms->ras = false;
> +    } else {
> +        /* Default disallows RAS instantiation */
> +        vms->ras = false;
> +        object_property_add_bool(obj, "ras", virt_get_ras,
> +                                 virt_set_ras, NULL);
> +        object_property_set_description(obj, "ras",
> +                                        "Set on/off to enable/disable "
> +                                        "RAS instantiation",
> +                                        NULL);
> +    }

For a property which is disabled by default, you don't need
to have a separate flag in the VirtMachineClass struct.
Those are only needed for properties where we need the old machine
types to have the property be 'off' but new machine types
need to default to it be 'on'. Since vms->ras is false
by default anyway, you can just have this part:

> +        /* Default disallows RAS instantiation */
> +        vms->ras = false;
> +        object_property_add_bool(obj, "ras", virt_get_ras,
> +                                 virt_set_ras, NULL);
> +        object_property_set_description(obj, "ras",
> +                                        "Set on/off to enable/disable "
> +                                        "RAS instantiation",
> +                                        NULL);

Compare the 'vms->secure' flag and associated property
for an example of this.

>      vms->irqmap = a15irqmap;
>
>      virt_flash_create(vms);
> @@ -2058,8 +2085,14 @@ DEFINE_VIRT_MACHINE_AS_LATEST(4, 2)
>
>  static void virt_machine_4_1_options(MachineClass *mc)
>  {
> +    VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
> +
>      virt_machine_4_2_options(mc);
>      compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
> +    /* Disable memory recovery feature for 4.1 as RAS support was
> +     * introduced with 4.2.
> +     */
> +    vmc->no_ras = true;
>  }
>  DEFINE_VIRT_MACHINE(4, 1)

thanks
-- PMM

Re: [PATCH v18 1/6] hw/arm/virt: Introduce RAS platform version and RAS machine option
Posted by Xiang Zheng 6 years, 4 months ago

On 2019/9/27 22:02, Peter Maydell wrote:
> On Fri, 6 Sep 2019 at 09:33, Xiang Zheng <zhengxiang9@huawei.com> wrote:
>>
>> From: Dongjiu Geng <gengdongjiu@huawei.com>
>>
>> Support RAS Virtualization feature since version 4.2, disable it by
>> default in the old versions. Also add a machine option which allows user
>> to enable it explicitly.
>>
>> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
>> Signed-off-by: Xiang Zheng <zhengxiang9@huawei.com>
>> ---
>>  hw/arm/virt.c         | 33 +++++++++++++++++++++++++++++++++
>>  include/hw/arm/virt.h |  2 ++
>>  2 files changed, 35 insertions(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index d74538b021..e0451433c8 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -1783,6 +1783,20 @@ static void virt_set_its(Object *obj, bool value, Error **errp)
>>      vms->its = value;
>>  }
>>
>> +static bool virt_get_ras(Object *obj, Error **errp)
>> +{
>> +    VirtMachineState *vms = VIRT_MACHINE(obj);
>> +
>> +    return vms->ras;
>> +}
>> +
>> +static void virt_set_ras(Object *obj, bool value, Error **errp)
>> +{
>> +    VirtMachineState *vms = VIRT_MACHINE(obj);
>> +
>> +    vms->ras = value;
>> +}
>> +
>>  static char *virt_get_gic_version(Object *obj, Error **errp)
>>  {
>>      VirtMachineState *vms = VIRT_MACHINE(obj);
>> @@ -2026,6 +2040,19 @@ static void virt_instance_init(Object *obj)
>>                                      "Valid values are none and smmuv3",
>>                                      NULL);
>>
>> +    if (vmc->no_ras) {
>> +        vms->ras = false;
>> +    } else {
>> +        /* Default disallows RAS instantiation */
>> +        vms->ras = false;
>> +        object_property_add_bool(obj, "ras", virt_get_ras,
>> +                                 virt_set_ras, NULL);
>> +        object_property_set_description(obj, "ras",
>> +                                        "Set on/off to enable/disable "
>> +                                        "RAS instantiation",
>> +                                        NULL);
>> +    }
> 
> For a property which is disabled by default, you don't need
> to have a separate flag in the VirtMachineClass struct.
> Those are only needed for properties where we need the old machine
> types to have the property be 'off' but new machine types
> need to default to it be 'on'. Since vms->ras is false
> by default anyway, you can just have this part:
> 
>> +        /* Default disallows RAS instantiation */
>> +        vms->ras = false;
>> +        object_property_add_bool(obj, "ras", virt_get_ras,
>> +                                 virt_set_ras, NULL);
>> +        object_property_set_description(obj, "ras",
>> +                                        "Set on/off to enable/disable "
>> +                                        "RAS instantiation",
>> +                                        NULL);
> 
> Compare the 'vms->secure' flag and associated property
> for an example of this.

Thanks for pointing it out, I will remove the no_ras in the VirtMachineClass struct.

> 
>>      vms->irqmap = a15irqmap;
>>
>>      virt_flash_create(vms);
>> @@ -2058,8 +2085,14 @@ DEFINE_VIRT_MACHINE_AS_LATEST(4, 2)
>>
>>  static void virt_machine_4_1_options(MachineClass *mc)
>>  {
>> +    VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
>> +
>>      virt_machine_4_2_options(mc);
>>      compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
>> +    /* Disable memory recovery feature for 4.1 as RAS support was
>> +     * introduced with 4.2.
>> +     */
>> +    vmc->no_ras = true;
>>  }
>>  DEFINE_VIRT_MACHINE(4, 1)
> 
> thanks
> -- PMM
> 
> .
> 

-- 

Thanks,
Xiang