Re: [Xen-devel] [PATCH V2] x86/altp2m: Hypercall to set altp2m view visibility

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.02.2020 15:42, Alexandru Stefan ISAILA wrote:
> 
>>>> --- a/xen/arch/x86/mm/hap/hap.c
>>>> +++ b/xen/arch/x86/mm/hap/hap.c
>>>> @@ -488,8 +488,17 @@ int hap_enable(struct domain *d, u32 mode)
>>>>                goto out;
>>>>            }
>>>>    
>>>> +        if ( (d->arch.altp2m_working_eptp = alloc_xenheap_page()) == NULL 
>>>> )
>>>> +        {
>>>> +            rv = -ENOMEM;
>>>> +            goto out;
>>>> +        }
>>>
>>> Isn't there a pre-existing error handling issue here which you
>>> widen, in that later encountered errors don't cause clean up
>>> of what had already succeeded before?
>>
>> It seems non of the errors perform a cleanup. It might be better to have
>> a general cleanup done at "out:" if ( !rv ) and then check what should
>> be "p2m_teardown()" or "free_xenheap_page()".
>>
> 
> I've looked around for this and it is handled in arch_domain_create().
> If hvm_domain_initialise() fails then paging_final_teardown() is called 
> and in the end hap_final_teardown() takes care of cleaning up.

Ah, good - thanks for checking. This code could be more obviously
correct, though.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel