On 06.09.2022 09:14, Penny Zheng wrote:
> Hi Jan
>
>> -----Original Message-----
>> From: Jan Beulich <jbeul...@suse.com>
>> Sent: Tuesday, September 6, 2022 2:34 PM
>> To: Penny Zheng <penny.zh...@arm.com>
>> Cc: Wei Chen <wei.c...@arm.com>; Andrew Cooper
>> <andrew.coop...@citrix.com>; George Dunlap <george.dun...@citrix.com>;
>> Julien Grall <jul...@xen.org>; Stefano Stabellini <sstabell...@kernel.org>;
>> Wei Liu <w...@xen.org>; xen-devel@lists.xenproject.org
>> Subject: Re: [PATCH v10 8/9] xen: retrieve reserved pages on
>> populate_physmap
>>
>> On 05.09.2022 09:08, Penny Zheng wrote:
>>> Hi jan
>>>
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeul...@suse.com>
>>>> Sent: Wednesday, August 17, 2022 6:05 PM
>>>> To: Penny Zheng <penny.zh...@arm.com>
>>>> Cc: Wei Chen <wei.c...@arm.com>; Andrew Cooper
>>>> <andrew.coop...@citrix.com>; George Dunlap
>>>> <george.dun...@citrix.com>; Julien Grall <jul...@xen.org>; Stefano
>>>> Stabellini <sstabell...@kernel.org>; Wei Liu <w...@xen.org>;
>>>> xen-devel@lists.xenproject.org
>>>> Subject: Re: [PATCH v10 8/9] xen: retrieve reserved pages on
>>>> populate_physmap
>>>>
>>>> On 16.08.2022 04:36, Penny Zheng wrote:
>>>>> @@ -2867,6 +2854,61 @@ int __init acquire_domstatic_pages(struct
>>>>> domain *d, mfn_t smfn,
>>>>>
>>>>> return 0;
>>>>> }
>>>>> +
>>>>> +/*
>>>>> + * Acquire nr_mfns contiguous pages, starting at #smfn, of static
>>>>> +memory,
>>>>> + * then assign them to one specific domain #d.
>>>>> + */
>>>>> +int __init acquire_domstatic_pages(struct domain *d, mfn_t smfn,
>>>>> + unsigned int nr_mfns, unsigned
>>>>> +int
>>>>> +memflags) {
>>>>> + struct page_info *pg;
>>>>> +
>>>>> + ASSERT_ALLOC_CONTEXT();
>>>>> +
>>>>> + pg = acquire_staticmem_pages(smfn, nr_mfns, memflags);
>>>>> + if ( !pg )
>>>>> + return -ENOENT;
>>>>> +
>>>>> + if ( assign_domstatic_pages(d, pg, nr_mfns, memflags) )
>>>>> + return -EINVAL;
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Acquire a page from reserved page list(resv_page_list), when
>>>>> +populating
>>>>> + * memory for static domain on runtime.
>>>>> + */
>>>>> +mfn_t acquire_reserved_page(struct domain *d, unsigned int
>>>>> +memflags) {
>>>>> + struct page_info *page;
>>>>> +
>>>>> + ASSERT_ALLOC_CONTEXT();
>>>>> +
>>>>> + /* Acquire a page from reserved page list(resv_page_list). */
>>>>> + spin_lock(&d->page_alloc_lock);
>>>>> + page = page_list_remove_head(&d->resv_page_list);
>>>>> + spin_unlock(&d->page_alloc_lock);
>>>>> + if ( unlikely(!page) )
>>>>> + return INVALID_MFN;
>>>>> +
>>>>> + if ( !prepare_staticmem_pages(page, 1, memflags) )
>>>>> + goto fail;
>>>>> +
>>>>> + if ( assign_domstatic_pages(d, page, 1, memflags) )
>>>>> + goto fail_assign;
>>>>> +
>>>>> + return page_to_mfn(page);
>>>>> +
>>>>> + fail_assign:
>>>>> + free_staticmem_pages(page, 1, memflags & MEMF_no_scrub);
>>>>
>>>> Doesn't this need to be !(memflags & MEMF_no_scrub)? And then - with
>>>
>>> I got a bit confused about this flag MEMF_no_scrub, does it mean no
>>> need to scrub?
>>
>> Yes, as its name says.
>>
>>> Since I saw that in alloc_domheap_pages(...)
>>> if ( assign_page(pg, order, d, memflags) )
>>> {
>>> free_heap_pages(pg, order, memflags & MEMF_no_scrub);
>>> return NULL;
>>> }
>>> It doesn't contain exclamation mark too...
>>
>> Hmm, you're right - on these error paths the scrubbing is needed if the page
>> wasn't previously scrubbed, as part of the set of pages may have been
>> transiently exposed to the guest (and by guessing it may have been able to
>> actually access the pages; I'm inclined to say it's its own fault though if
>> that
>> way information is being leaked).
>>
>
> Then, the same for the acquire_domstatic_pages(...)
>
> if ( assign_pages(pg, nr_mfns, d, memflags) )
> {
> free_staticmem_pages(pg, nr_mfns, memflags & MEMF_no_scrub);
> return -EINVAL;
> }
> On this error path, it has misused the MEMF_no_scrub too.
Why do you say "misused"?
> But IMO, as we are talking about these pages will always be reserved to the
> guest,
> maybe here it also doesn't need scrubbing at all?
Perhaps. It feels as if we had been there before, quite some time ago.
Jan
