Re: [PATCH v4 3/3] xen: introduce CONFIG_HAS_DOMAIN_TYPE

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 5/6/26 10:10 AM, Jan Beulich wrote:
> On 06.05.2026 09:44, Oleksii Kurochko wrote:
> > On 5/4/26 2:21 PM, Jan Beulich wrote:
> > > On 27.04.2026 17:34, Oleksii Kurochko wrote:
> > > > @@ -69,6 +70,14 @@ static inline uint32_t alloc_phandle(struct kernel_info 
> > > > *kinfo)
> > > >        return kinfo->next_phandle >= GUEST_PHANDLE_GIC ? 0 : 
> > > > kinfo->next_phandle++;
> > > >    }
> > > >    
> > > > +static inline void set_domain_type(struct domain *d, struct kernel_info *kinfo)
> > > Pointer-to-const for the 2nd parameter?
> > I will apply this comment and comment above.
> >
> > > > +{
> > > > +#ifdef CONFIG_HAS_DOMAIN_TYPE
> > > > +    /* Type must be set before allocate memory */
> > > This comment would be more prominent if it lived outside of the #ifdef,
> > > perhaps (read on) ahead of the function. I wonder though why it's only
> > > a comment, and not e.g. an assertion. If an assertion was possible to
> > > add, the comment would want to live next to it. Without an assertion
> > > putting it ahead of the function may be better.
> > >
> > > Depending on how far to go, changes could be made while committing, or a
> > > proper v5 may want submitting.
> > I think that instead of comment or just after comment the following
> > could be added:
> >     ASSERT(!domain_tot_pages(d));
> >
> > Jan, Michal, do you see any concern with that ASSERT() or I could add it
> > and keep your Ack-by and R-by.
> I'm okay with it being added, as long as you have made sure that it is
> legitimate to have. IOW (as pointed out numerous times before) you may
> not assert on state that's user/admin controlled, and that isn't covered
> by another, earlier check. In such a case an error would need returning
> instead.
All callers of set_domain_type() are in the domain build path, before 
any memory allocation, so domain_tot_pages(d) being zero at this point 
is an internal code invariant, not user-controlled.
Also, I checked CI and it looks okay except ARM64 randcondig I mentioned 
in xen-devel matrix channel:
https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2503536959

~ Oleksii