| Message ID | 1541672989-15967-8-git-send-email-gengdongjiu@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add ARMv8 RAS virtualization support in QEMU | expand |
On 8 November 2018 at 10:29, Dongjiu Geng <gengdongjiu@huawei.com> 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@huawei.com> > --- > 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
Hi Peter, Thanks very much for the commands and review. > On 8 November 2018 at 10:29, Dongjiu Geng <gengdongjiu@huawei.com> 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@huawei.com> > > --- > > 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
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index 4880a05..003deac 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -625,6 +625,39 @@ int kvm_vm_check_extension(KVMState *s, unsigned int extension) return ret; } +typedef struct HWPoisonPage { + ram_addr_t ram_addr; + QLIST_ENTRY(HWPoisonPage) list; +} HWPoisonPage; + +static QLIST_HEAD(, HWPoisonPage) hwpoison_page_list = + QLIST_HEAD_INITIALIZER(hwpoison_page_list); + +void kvm_unpoison_all(void *param) +{ + HWPoisonPage *page, *next_page; + + QLIST_FOREACH_SAFE(page, &hwpoison_page_list, list, next_page) { + QLIST_REMOVE(page, list); + qemu_ram_remap(page->ram_addr, TARGET_PAGE_SIZE); + g_free(page); + } +} + +void kvm_hwpoison_page_add(ram_addr_t ram_addr) +{ + HWPoisonPage *page; + + QLIST_FOREACH(page, &hwpoison_page_list, list) { + if (page->ram_addr == ram_addr) { + return; + } + } + page = g_new(HWPoisonPage, 1); + page->ram_addr = ram_addr; + QLIST_INSERT_HEAD(&hwpoison_page_list, page, list); +} + static uint32_t adjust_ioeventfd_endianness(uint32_t val, uint32_t size) { #if defined(HOST_WORDS_BIGENDIAN) != defined(TARGET_WORDS_BIGENDIAN) diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h index 9ecd911..1e02fb9 100644 --- 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); + #define DIRTY_CLIENTS_ALL ((1 << DIRTY_MEMORY_NUM) - 1) #define DIRTY_CLIENTS_NOCODE (DIRTY_CLIENTS_ALL & ~(1 << DIRTY_MEMORY_CODE)) diff --git a/target/i386/kvm.c b/target/i386/kvm.c index 796a049..08b3feb 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -457,39 +457,6 @@ uint32_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index) } -typedef struct HWPoisonPage { - ram_addr_t ram_addr; - QLIST_ENTRY(HWPoisonPage) list; -} HWPoisonPage; - -static QLIST_HEAD(, HWPoisonPage) hwpoison_page_list = - QLIST_HEAD_INITIALIZER(hwpoison_page_list); - -static void kvm_unpoison_all(void *param) -{ - HWPoisonPage *page, *next_page; - - QLIST_FOREACH_SAFE(page, &hwpoison_page_list, list, next_page) { - QLIST_REMOVE(page, list); - qemu_ram_remap(page->ram_addr, TARGET_PAGE_SIZE); - g_free(page); - } -} - -static void kvm_hwpoison_page_add(ram_addr_t ram_addr) -{ - HWPoisonPage *page; - - QLIST_FOREACH(page, &hwpoison_page_list, list) { - if (page->ram_addr == ram_addr) { - return; - } - } - page = g_new(HWPoisonPage, 1); - page->ram_addr = ram_addr; - QLIST_INSERT_HEAD(&hwpoison_page_list, page, list); -} - static int kvm_get_mce_cap_supported(KVMState *s, uint64_t *mce_cap, int *max_banks) {
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@huawei.com> --- 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 --- accel/kvm/kvm-all.c | 33 +++++++++++++++++++++++++++++++++ include/exec/ram_addr.h | 5 +++++ target/i386/kvm.c | 33 --------------------------------- 3 files changed, 38 insertions(+), 33 deletions(-)