Re: [RFC XEN PATCH v2 3/3] tools: Add new function to get gsi from irq

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Dec 04, 2023 at 05:38:06AM +0000, Chen, Jiqian wrote:
> On 2023/12/1 17:03, Roger Pau Monné wrote:
> > On Thu, Nov 30, 2023 at 07:09:12PM -0800, Stefano Stabellini wrote:
> >> On Thu, 30 Nov 2023, Roger Pau Monné wrote:
> >>> On Wed, Nov 29, 2023 at 08:02:40PM -0800, Stefano Stabellini wrote:
> >>>> n Wed, 29 Nov 2023, Stefano Stabellini wrote:
> >>>>> On Tue, 28 Nov 2023, Roger Pau Monné wrote:
> >>>>>> On Fri, Nov 24, 2023 at 06:41:36PM +0800, Jiqian Chen wrote:
> >>>>>>> In PVH dom0, it uses the linux local interrupt mechanism,
> >>>>>>> when it allocs irq for a gsi, it is dynamic, and follow
> >>>>>>> the principle of applying first, distributing first. And
> >>>>>>> if you debug the kernel codes, you will find the irq
> >>>>>>> number is alloced from small to large, but the applying
> >>>>>>> gsi number is not, may gsi 38 comes before gsi 28, that
> >>>>>>> causes the irq number is not equal with the gsi number.
> >>>>>>> And when we passthrough a device, QEMU will use its gsi
> >>>>>>> number to do mapping actions, see xen_pt_realize->
> >>>>>>> xc_physdev_map_pirq, but the gsi number is got from file
> >>>>>>> /sys/bus/pci/devices/xxxx:xx:xx.x/irq in current code,
> >>>>>>> so it will fail when mapping.
> >>>>>>> And in current codes, there is no method to translate
> >>>>>>> irq to gsi for userspace.
> >>>>>>
> >>>>>> I think it would be cleaner to just introduce a new sysfs node that
> >>>>>> contains the gsi if a device is using one (much like the irq sysfs
> >>>>>> node)?
> >>>>>>
> >>>>>> Such ioctl to translate from IRQ to GSI has nothing to do with Xen, so
> >>>>>> placing it in privcmd does seem quite strange to me.  I understand
> >>>>>> that for passthrough we need the GSI, but such translation layer from
> >>>>>> IRQ to GSI is all Linux internal, and it would be much simpler to just
> >>>>>> expose the GSI in sysfs IMO.
> >>>>>
> >>>>> Maybe something to add to drivers/xen/sys-hypervisor.c in Linux.
> >>>>> Juergen what do you think?
> >>>>
> >>>> Let me also add that privcmd.c is already a Linux specific interface.
> >>>> Although it was born to be a Xen hypercall "proxy" in reality today we
> >>>> have a few privcmd ioctls that don't translate into hypercalls. So I
> >>>> don't think that adding IOCTL_PRIVCMD_GSI_FROM_IRQ would be a problem.
> >>>
> >>> Maybe not all ioctls translate to hypercalls (I guess you are
> >>> referring to the IOCTL_PRIVCMD_RESTRICT ioctl), but they are specific
> >>> Xen actions.  Getting the GSI used by a device has nothing do to with
> >>> Xen.
> >>>
> >>> IMO drivers/xen/sys-hypervisor.c is also not appropriate, but I'm not
> >>> the maintainer of any of those components.
> >>>
> >>> There's nothing Xen specific about fetching the GSI associated with a
> >>> PCI device.  The fact that Xen needs it for passthrough is just a red
> >>> herring, further cases where the GSI is needed might arise outside of
> >>> Xen, and hence such node would better be placed in a generic
> >>> location.  The right location should be /sys/bus/pci/devices/<sbdf>/gsi.
> >>
> >> That might be true but /sys/bus/pci/devices/<sbdf>/gsi is a non-Xen
> >> generic interface and the maintainers of that portion of Linux code
> >> might have a different opinion. We'll have to see.
> > 
> > Right, but before resorting to implement a Xen specific workaround
> > let's attempt to do it the proper way :).
> > 
> > I cannot see why exposing the gsi on sysfs like that would be an
> > issue.  There's a lot of resource information exposed on sysfs
> > already, and it's a trivial node to implement.
> Thanks for both of you' s suggestions. At present, it seems the result of 
> discussion is that it needs to add a gsi sysfs. I will modify it in the next 
> version and then add the corresponding maintainer to the review list.

Thanks, please keep xen-devel on Cc if possible.  Maybe if the
suggested path is not suitable maintainers can recommend another path
where the gsi (or equivalent) node could live.

Thanks, Roger.