Re: [Xen-devel] [PATCH v2] mm: Don't scrub pages while holding heap lock in alloc_heap_pages()

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

>> @@ -974,13 +972,39 @@ static struct page_info *alloc_heap_pages(
>>           * guest can control its own visibility of/through the cache.
>>           */
>>          flush_page_to_ram(page_to_mfn(&pg[i]), !(memflags & 
>> MEMF_no_icache_flush));
>> -
>> -        if ( !(memflags & MEMF_no_scrub) )
>> -            check_one_page(&pg[i]);
>>      }
>>  
>>      spin_unlock(&heap_lock);
>>  
>> +    if ( first_dirty != INVALID_DIRTY_IDX ||
>> +         (scrub_debug && !(memflags & MEMF_no_scrub)) )
> Why does scrub_debug matter here?.
>
>> +    {
>> +        for ( i = 0; i < (1U << order); i++ )
>> +        {
>> +            if ( test_bit(_PGC_need_scrub, &pg[i].count_info) )
>> +            {
>> +                if ( !(memflags & MEMF_no_scrub) )
>> +                    scrub_one_page(&pg[i]);
>> +
>> +                dirty_cnt++;
>> +
>> +                spin_lock(&heap_lock);
>> +                pg[i].count_info &= ~PGC_need_scrub;
>> +                spin_unlock(&heap_lock);
>> +            }
>> +
>> +            if ( !(memflags & MEMF_no_scrub) )
>> +                check_one_page(&pg[i]);
> Wouldn't this better be "else if", as checking a page just scrubbed
> doesn't look very useful?

For both of these questions --- we don't want to miss a poisoned page.
For example, if a page was poisoned but for some reason is not marked
PGC_need_scrub.

Of course, we risk a false positive if a guest wrote the page with the
same pattern.

-boris

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel