"Jan Beulich" <jbeul...@suse.com> writes:
>>>> On 17.05.17 at 18:15, <punit.agra...@arm.com> wrote:
>> Jan Beulich <jbeul...@suse.com> writes:
>>
>>>>>> On 15.05.17 at 16:10, <punit.agra...@arm.com> wrote:
>>>> --- a/xen/include/asm-x86/page.h
>>>> +++ b/xen/include/asm-x86/page.h
>>>> @@ -375,6 +375,10 @@ perms_strictly_increased(uint32_t old_flags, uint32_t
>>>> new_flags)
>>>>
>>>> #define PAGE_ALIGN(x) (((x) + PAGE_SIZE - 1) & PAGE_MASK)
>>>>
>>>> +static inline void invalidate_icache(void)
>>>> +{
>>>> +}
>>>
>>> This function clearly does not what its name says, so there should
>>> be a brief comment saying why.
>>
>> Ack. I've added the following comment block above the function
>> definition.
>>
>> /*
>> * While allocating memory for a domain, invalidate_icache() is called
>> * to ensure that guest vcpu does not execute any stale instructions
>> * from the recently allocated memory. There is nothing to be done
>> * here as icaches are coherent on x86.
>> */
>>
>> My x86 foo is weak and I'd appreciate if somebody familiar with x86
>> could give this a once over.
>
> The text looks okay, but it ties the function to its _current_ single
> user. I'd like to ask you to rather use just the last sentence, and it's
> questionable (a matter of taste mostly) whether such a comment
> wouldn't better be placed inside the function. And perhaps it would
> be a good idea to insert "sufficiently", as they're not fully coherent
> (see the self modifying code constraints on MP systems).
I've updated the text and moved it inside the function.
Thanks for the feedback!
>
> Jan
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
