On 30.04.2024 18:00, Petr Beneš wrote:
> On Tue, Apr 30, 2024 at 4:27 PM Jan Beulich <jbeul...@suse.com> wrote:
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -685,6 +685,12 @@ int arch_sanitise_domain_config(struct
>>> xen_domctl_createdomain *config)
>>> return -EINVAL;
>>> }
>>>
>>> + if ( config->max_altp2m > MAX_EPTP )
>>> + {
>>> + dprintk(XENLOG_INFO, "max_altp2m must be <= %u\n", MAX_EPTP);
>>> + return -EINVAL;
>>> + }
>>
>> ... using MAX_EPTP here feels like a layering violation to me. Imo there want
>> to be separate constants, tied together with a suitably placed
>> BUILD_BUG_ON().
>>
>> Furthermore comparisons like this (there are further ones elsewhere) suggest
>> there is a (continued) naming issue: A max_ or MAX_ prefix ought to name a
>> "maximum valid value", not "number of permitted values". This is not a
>> request to alter MAX_EPTP, but one to make sure the new struct fields really
>> have matching names and purposes.
>
> Do you have any proposals? I was considering nr_altp2m. Another
> question is what it should be named in xl.cfg - also nr_altp2m? I was
> too hesitant to name it like that, since there aren't any nr_* fields
> currently.
Internally nr_ or num_ are going to be fine. For xl whether either of
those would be, or maybe altp2ms= (along the lines of e.g. vcpus=), or
altp2m_count, or yet something else I simply don't know. That'll be
the maintainers there to help with.
Jan
