On 28.08.2024 11:53, [email protected] wrote:
> On Tue, 2024-08-27 at 12:29 +0200, Jan Beulich wrote:
>>>
>>> +
>>> +/*
>>> + * Direct access to xen_fixmap[] should only happen when {set,
>>> + * clear}_fixmap() is unusable (e.g. where we would end up to
>>> + * recursively call the helpers).
>>> + */
>>> +extern pte_t xen_fixmap[];
>>
>> I'm afraid I keep being irritated by the comment: What recursive use
>> of
>> helpers is being talked about here? I can't see anything recursive in
>> this
>> patch. If this starts happening with a subsequent patch, then you
>> have
>> two options: Move the declaration + comment there, or clarify in the
>> description (in enough detail) what this is about.
> This comment is added because of:
> ```
> void *__init pmap_map(mfn_t mfn)
> ...
> /*
> * We cannot use set_fixmap() here. We use PMAP when the domain map
> * page infrastructure is not yet initialized, so
> map_pages_to_xen() called
> * by set_fixmap() needs to map pages on demand, which then calls
> pmap()
> * again, resulting in a loop. Modify the PTEs directly instead.
> The same
> * is true for pmap_unmap().
> */
> arch_pmap_map(slot, mfn);
> ...
> ```
> And it happens because set_fixmap() could be defined using generic PT
> helpers
As you say - could be. If I'm not mistaken no set_fixmap() implementation
exists even by the end of the series. Fundamentally I'd expect set_fixmap()
to (possibly) use xen_fixmap[] directly. That in turn ...
> so what will lead to recursive behaviour when when there is no
> direct map:
... would mean no recursion afaict. Hence why clarification is needed as
to what's going on here _and_ what's planned.
Jan
> static pte_t *map_table(mfn_t mfn)
> {
> /*
> * During early boot, map_domain_page() may be unusable. Use the
> * PMAP to map temporarily a page-table.
> */
> if ( system_state == SYS_STATE_early_boot )
> return pmap_map(mfn);
> ...
> }
