Re: [Qemu-arm] [PATCH RESEND v15 07/10] KVM: Move related hwpoison page functions to accel/kvm/ folder

[Peter Maydell]

On 8 November 2018 at 10:29, Dongjiu Geng <address@hidden> wrote:
> kvm_hwpoison_page_add() and kvm_unpoison_all() will be used both
> by X86 and ARM platforms, so move these functions to a common
> accel/kvm/ folder to avoid duplicate code.
>
> Signed-off-by: Dongjiu Geng <address@hidden>
> ---
> Address Peter's comments to move related hwpoison page function to
> accel/kvm folder in [1]
> Address Paolo's comments to move HWPoisonPage definition back to
> accel/kvm/kvm-all.c
>
> [1]:
> https://lists.gnu.org/archive/html/qemu-arm/2017-09/msg00077.html
> https://lists.gnu.org/archive/html/qemu-arm/2017-09/msg00152.html
> ---

> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -115,6 +115,11 @@ void qemu_ram_free(RAMBlock *block);
>
>  int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp);
>
> +/* Add a poisoned page to the list */
> +void kvm_hwpoison_page_add(ram_addr_t ram_addr);
> +/* Free and remove all the poisoned pages in the list */
> +void kvm_unpoison_all(void *param);
> +

In my previous review comments I said:
>>> Any new globally-visible function prototype in a header should
>>> have a doc-comment formatted documentation comment, please.
>>
>>
>> Ok, thanks for this reminder. Do you mean I need to add comments
>> for this globally-visible function, such as below:
>>
>> /*
>>  * xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
>>  */
>> void kvm_hwpoison_page_add(ram_addr_t ram_addr);
>
> It should be in the doc-comment format, which begins
> "/**" and has some stylization of how you list parameters
> and so on. Lots of examples in the existing headers.

Can we have a doc-comment in the proper format and with a little
more detail than a single line, please?

thanks
-- PMM