On 27.02.2024 14:24, Henry Wang wrote:
> On 2/26/2024 4:25 PM, Jan Beulich wrote:
>> On 26.02.2024 02:19, Henry Wang wrote:
>>> --- a/xen/common/memory.c
>>> +++ b/xen/common/memory.c
>>> @@ -219,7 +219,7 @@ static void populate_physmap(struct memop_args *a)
>>> }
>>> else
>>> {
>>> - if ( is_domain_direct_mapped(d) )
>>> + if ( is_domain_direct_mapped(d) && !is_magic_gpfn(gpfn) )
>>> {
>>> mfn = _mfn(gpfn);
>>>
>> I wonder whether is_domain_direct_mapped() shouldn't either be cloned
>> into e.g. is_gfn_direct_mapped(d, gfn), or be adjusted in-place to gain
>> such a (then optional) 2nd parameter. (Of course there again shouldn't be
>> a need for every domain to define a stub is_domain_direct_mapped() - if
>> not defined by an arch header, the stub can be supplied in a single
>> central place.)
>
> Same here, it looks like you prefer the centralized
> is_domain_direct_mapped() now, as we are having more archs. I can do the
> clean-up when sending v2. Just out of curiosity, do you think it is a
> good practice to place the is_domain_direct_mapped() implementation in
> xen/domain.h with proper arch #ifdefs?
arch #ifdefs? I'd like to avoid such, if at all possible. Generic fallbacks
generally ought to key their conditionals to the very identifier not
(already) being defined.
Jan
