Re: [PATCH v4 5/6] xen/x86: add PHYSDEVOP_msi_set_enable

From: Marek Marczykowski <marmarek@invisiblethingslab.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	Julien Grall <julien.grall@arm.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Daniel de Graaf <dgdegra@tycho.nsa.gov>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH v4 5/6] xen/x86: add PHYSDEVOP_msi_set_enable
Date: Sun, 3 Mar 2019 04:26:29 +0100	[thread overview]
Message-ID: <20190303032629.GQ19265@mail-itl> (raw)
In-Reply-To: <5C77BEDD020000780021B030@prv1-mh.provo.novell.com>

[-- Attachment #1.1: Type: text/plain, Size: 1616 bytes --]

On Thu, Feb 28, 2019 at 03:58:37AM -0700, Jan Beulich wrote:
> >>> On 27.02.19 at 16:05, <marmarek@invisiblethingslab.com> wrote:
> > On Wed, Feb 27, 2019 at 04:41:37AM -0700, Jan Beulich wrote:
> >> >>> On 07.02.19 at 01:07, <marmarek@invisiblethingslab.com> wrote:
> >> > +int msi_msix_set_enable(struct pci_dev *pdev, int mode, int enable)
> >> > +{
> >> > +    int ret;
> >> > +
> >> > +    ret = xsm_msi_set_enable(XSM_DM_PRIV, pdev->domain,
> >> > +                             (pdev->seg << 16) | (pdev->bus << 8) | pdev->devfn,
> >> > +                             mode, enable);
> >> > +    if ( ret )
> >> > +        return ret;
> >> > +
> >> > +    switch ( mode )
> >> > +    {
> >> > +    case PHYSDEVOP_MSI_SET_ENABLE_MSI:
> >> > +        msi_set_enable(pdev, enable);
> >> > +        break;
> >> > +
> >> > +    case PHYSDEVOP_MSI_SET_ENABLE_MSIX:
> >> > +        msix_set_enable(pdev, enable);
> >> > +        break;
> >> > +    }
> >> 
> >> What about a call to pci_intx()? 
> > 
> > Should pci_intx(dev, !enable) be called in all those cases?
> 
> Well, that depends whether Dom0 is involved, which is where the
> operation would normally be done. But since this is about bypassing
> pciback, I think it may be needed.

Shouldn't that be done by device model itself? Or even on command from
the target domain? Automatically toggling INTx when manipulating MSI(-X)
seems wrong.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel