Re: [PATCH for-4.19???] x86/physdev: replace physdev_{,un}map_pirq() checking against DOMID_SELF

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 12/06/2024 9:44 am, Jan Beulich wrote:
> It's hardly ever correct to check for just DOMID_SELF, as guests have
> ways to figure out their domain IDs and hence could instead use those as
> inputs to respective hypercalls. Note, however, that for ordinary DomU-s
> the adjustment is relaxing things rather than tightening them, since
> - as a result of XSA-237 - the respective XSM checks would have rejected
> self (un)mapping attempts for other than the control domain.
>
> Since in physdev_map_pirq() handling overall is a little easier this
> way, move obtaining of the domain pointer into the caller. Doing the
> same for physdev_unmap_pirq() is just to keep both consistent in this
> regard. For both this has the advantage that it is now provable (by the
> build not failing) that there are no DOMID_SELF checks left (and none
> could easily be re-added).
>
> Fixes: 0b469cd68708 ("Interrupt remapping to PIRQs in HVM guests")
> Fixes: 9e1a3415b773 ("x86: fixes after emuirq changes")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

I think it is right to perform the domid lookup in do_physdev_op() and
pass d down into physdev_{un,}map_pirq().

But I don't see what this has to do with the build failing.  You're not
undef-ing DOMID_SELF, so I don't see what kind of provability you've added.

> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -184,6 +170,8 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
>  
>      switch ( cmd )
>      {
> +        struct domain *d;
> +

Please don't introduce any more of these.

We've discussed several times about wanting to start using trivial
autovar init support, and every one of these additions is going to need
reverting.

In this case, there's literally no difference having it at function scope.

Furthermore, I recall it being a MISRA violation now too.

~Andrew