On 09.10.2025 13:34, Oleksii Kurochko wrote:
> On 10/7/25 3:25 PM, Jan Beulich wrote:
>> On 01.10.2025 18:00, Oleksii Kurochko wrote:
>>> On 9/23/25 12:41 AM, Jan Beulich wrote:
>>>> On 17.09.2025 23:55, Oleksii Kurochko wrote:
>>>>> + if ( *md_pg )
>>>>> + metadata = __map_domain_page(*md_pg);
>> Not this conditional assignment for ...
>>
>>>>> + if ( t < p2m_first_external )
>>>>> + {
>>>>> pte->pte |= MASK_INSR(t, P2M_TYPE_PTE_BITS_MASK);
>>>>>
>>>>> - return rc;
>>>>> + if ( metadata )
>>>>> + metadata[ctx->index].pte = p2m_invalid;
>>>>> + }
>>>>> + else
>>>>> + {
>>>>> + pte->pte |= MASK_INSR(p2m_ext_storage, P2M_TYPE_PTE_BITS_MASK);
>>>>> +
>>>>> + metadata[ctx->index].pte = t;
>>>> Afaict metadata can still be NULL when you get here.
>>> It shouldn't be, because when this line is executed, the metadata page
>>> already
>>> exists or was allocated at the start of p2m_set_type().
>> ... this reply of yours. And the condition there can be false, in case you
>> took the domain_crash() path.
>
> Oh, right, for some reason, I thought we didn’t return from|domain_crash()|.
> I’m curious whether calling|domain_crash()| might break something, as some
> useful
> data could be freed and negatively affect the internals
> of|map_regions_p2mt()|.
>
> It might make more sense to use|panic()| here instead.
> Do you have any thoughts or suggestions on this?
domain_crash() is generally preferable over crashing the system as a whole.
I don't follow what negative effects you're alluding to. Did you look at
what domain_crash() does? It doesn't start tearing down the domain, that'll
still need invoking from the toolstack. A crashed domain will stay around
with all its resources allocated.
Jan