Am 14.01.2011 02:51, Huang Ying wrote:
> On Thu, 2011-01-13 at 17:01 +0800, Jan Kiszka wrote:
>> Am 13.01.2011 09:34, Huang Ying wrote:
>>> In Linux kernel HWPoison processing implementation, the virtual
>>> address in processes mapping the error physical memory page is marked
>>> as HWPoison. So that, the further accessing to the virtual
>>> address will kill corresponding processes with SIGBUS.
>>>
>>> If the error physical memory page is used by a KVM guest, the SIGBUS
>>> will be sent to QEMU, and QEMU will simulate a MCE to report that
>>> memory error to the guest OS. If the guest OS can not recover from
>>> the error (for example, the page is accessed by kernel code), guest OS
>>> will reboot the system. But because the underlying host virtual
>>> address backing the guest physical memory is still poisoned, if the
>>> guest system accesses the corresponding guest physical memory even
>>> after rebooting, the SIGBUS will still be sent to QEMU and MCE will be
>>> simulated. That is, guest system can not recover via rebooting.
>>>
>>> In fact, across rebooting, the contents of guest physical memory page
>>> need not to be kept. We can allocate a new host physical page to
>>> back the corresponding guest physical address.
>>>
>>> This patch fixes this issue in QEMU via calling qemu_ram_remap() to
>>> clear the corresponding page table entry, so that make it possible to
>>> allocate a new page to recover the issue.
>>>
>>> Signed-off-by: Huang Ying <ying.huang@xxxxxxxxx>
>>> ---
>>> kvm.h | 2 ++
>>> target-i386/kvm.c | 39 +++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 41 insertions(+)
>>>
>>> --- a/target-i386/kvm.c
>>> +++ b/target-i386/kvm.c
>>> @@ -580,6 +580,42 @@ static int kvm_get_supported_msrs(void)
>>> return ret;
>>> }
>>>
>>> +struct HWPoisonPage;
>>> +typedef struct HWPoisonPage HWPoisonPage;
>>> +struct HWPoisonPage
>>> +{
>>> + ram_addr_t ram_addr;
>>> + QLIST_ENTRY(HWPoisonPage) list;
>>> +};
>>> +
>>> +static QLIST_HEAD(hwpoison_page_list, HWPoisonPage) hwpoison_page_list =
>>> + QLIST_HEAD_INITIALIZER(hwpoison_page_list);
>>> +
>>> +void kvm_unpoison_all(void *param)
>>
>> Minor nit: This can be static now.
>
> In uq/master, it can be make static. But in kvm/master, kvm_arch_init
> is not compiled because of conditional compiling, so we will get warning
> and error for unused symbol. Should we consider kvm/master in this
> patch?
qemu-kvm is very close to switching to upstream kvm_*init. As long as it
requires this service in its own modules, it will have to patch this
detail. It does this for other functions already.
>
>>> +{
>>> + 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);
>>> + qemu_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 = qemu_malloc(sizeof(HWPoisonPage));
>>> + page->ram_addr = ram_addr;
>>> + QLIST_INSERT_HEAD(&hwpoison_page_list, page, list);
>>> +}
>>> +
>>> int kvm_arch_init(void)
>>> {
>>> uint64_t identity_base = 0xfffbc000;
>>> @@ -632,6 +668,7 @@ int kvm_arch_init(void)
>>> fprintf(stderr, "e820_add_entry() table is full\n");
>>> return ret;
>>> }
>>> + qemu_register_reset(kvm_unpoison_all, NULL);
>>>
>>> return 0;
>>> }
>>> @@ -1940,6 +1977,7 @@ int kvm_on_sigbus_vcpu(CPUState *env, in
>>> hardware_memory_error();
>>> }
>>> }
>>> + kvm_hwpoison_page_add(ram_addr);
>>>
>>> if (code == BUS_MCEERR_AR) {
>>> /* Fake an Intel architectural Data Load SRAR UCR */
>>> @@ -1984,6 +2022,7 @@ int kvm_on_sigbus(int code, void *addr)
>>> "QEMU itself instead of guest system!: %p\n", addr);
>>> return 0;
>>> }
>>> + kvm_hwpoison_page_add(ram_addr);
>>> kvm_mce_inj_srao_memscrub2(first_cpu, paddr);
>>> } else
>>> #endif
>>> --- a/kvm.h
>>> +++ b/kvm.h
>>> @@ -188,6 +188,8 @@ int kvm_physical_memory_addr_from_ram(ra
>>> target_phys_addr_t *phys_addr);
>>> #endif
>>>
>>> +void kvm_unpoison_all(void *param);
>>> +
>>
>> To be removed if kvm_unpoison_all is static.
>>
>>> #endif
>>> int kvm_set_ioeventfd_mmio_long(int fd, uint32_t adr, uint32_t val, bool assign);
>>>
>>>
>>
>> As indicated, I'm sitting on lots of fixes and refactorings of the MCE
>> user space code. How do you test your patches? Any suggestions how to do
>> this efficiently would be warmly welcome.
>
> We use a self-made test script to test. Repository is at:
>
> git://git.kernel.org/pub/scm/utils/cpu/mce/mce-test.git
>
> The kvm test script is in kvm sub-directory.
>
> The qemu patch attached is need by the test script.
>
Yeah, I already found this yesterday and started reading. I was just
searching for p2v in qemu, but now it's clear where it comes from. Will
have a look (if you want to preview my changes:
git://git.kiszka.org/qemu-kvm.git queues/kvm-upstream).
I was almost about to use MADV_HWPOISON instead of the injection module.
Is there a way to recover the fake corruption afterward? I think that
would allow to move some of the test logic into qemu and avoid p2v which
- IIRC - was disliked upstream.
Also, is there a way to simulate corrected errors (BUS_MCEERR_AO)?
Thanks,
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
