On 2019/9/27 22:02, Peter Maydell wrote:
> On Fri, 6 Sep 2019 at 09:33, Xiang Zheng <zhengxiang9@xxxxxxxxxx> wrote:
>>
>> From: Dongjiu Geng <gengdongjiu@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>> Signed-off-by: Xiang Zheng <zhengxiang9@xxxxxxxxxx>
>> ---
>> 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
