Hi Peter, Thanks very much for the commands and review. > On 8 November 2018 at 10:29, Dongjiu Geng <gengdongjiu@xxxxxxxxxx> 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 <gengdongjiu@xxxxxxxxxx> > > --- > > 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? Sure, I will add it in the proper format, thanks, May be I forget it. > > thanks > -- PMM