Re: [PATCH 20/24] XSM: fold xsm_{,un}map_domain_pirq() hooks

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 8/13/26 9:02 AM, Jan Beulich wrote:
> On 13.08.2026 14:42, Daniel P. Smith wrote:
> > On 8/6/26 3:36 AM, Jan Beulich wrote:
> > > On 06.08.2026 03:10, Daniel P. Smith wrote:
> > > > On 7/28/26 9:23 AM, Jan Beulich wrote:
> > > > > --- a/xen/xsm/flask/hooks.c
> > > > > +++ b/xen/xsm/flask/hooks.c
> > > > > @@ -1022,14 +1022,9 @@ static char *cf_check flask_show_irq_sid
> > > > >     
> > > > >     #ifdef CONFIG_HAS_PIRQ
> > > > >     
> > > > > -static int cf_check flask_map_domain_pirq(struct domain *d)
> > > > > +static int cf_check flask_map_domain_pirq(struct domain *d, bool access)
> > > > >     {
> > > > > -    return current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__ADD);
> > > > > -}
> > > > > -
> > > > > -static int cf_check flask_unmap_domain_pirq(struct domain *d)
> > > > > -{
> > > > > -    return current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__REMOVE);
> > > > > +    return current_has_perm(d, SECCLASS_RESOURCE, resource_to_perm(access));
> > > > >     }
> > > > >     
> > > > >     #endif /* CONFIG_HAS_PIRQ */
> > > > I am not opposed to collapsing the calls as long as the semantic is not
> > > > lost, which I feel the reuse of the xsm_map_domain_pirq does looses it
> > > > much less provides an opportunity for confusion. Something like
> > > > xsm_domain_pirq(..., access) makes more semantic sense to me, as it
> > > > would read, grant domain pirq access T/F.
> > > In fact I was as well wondering about naming (and a possible name change)
> > > here. I decided against it because of the other hooks (touched by patch
> > > 19), which all follow a similar model of their names really more expressing
> > > the "positive" form than the "negative" one. Further, entirely losing "map"
> > > from the name also doesn't look quite right, as physdev_{,un}map_pirq() is
> > > where they're called from. To follow the naming of some of the other hook,
> > > maybe xsm_domain_pirq_mapping(..., bool map)? Possibly even with "domain"
> > > dropped from the name, fully fitting xsm_io{mem,port}_mapping()? Which
> > > would then further raise the question whether patch 19 should maybe rename
> > > the parameters of those two hooks to "map" at the same time.
> > Sound like you and I were running through the same argument with
> > ourselves, except you went right and I went left.
> >
> > For me, I find the false case of xsm_map_domain_pirq(..., access) reads
> > a bit weird, "Can the target map domain pirq access=false?" I do like
> > your mapping suggestion along with the parameter rename to map. It
> > changes the reading to "Is target allowed map(true)/unmap(false) for the
> > domain pirq mapping?" This could be carried through to the other cases.
> Will do. As to patch 19: I'm inclined to leave the parameters as "allow"
> in dummy.h, but change at least the flask_io{mem,port}_mapping() ones to
> "map". Will that be okay with you? Else what scheme would you prefer?
I am good with that. Those weren't bad, it was this one that I found 
that the original name really didn't work well with the collapse.
v/r,
dps