linux-kernel - Re: [PATCH RFC v2 03/18] irq/dev-msi: Create IR-DEV-MSI irq domain

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200722173348.GK2021248@mellanox.com>
Date:   Wed, 22 Jul 2020 14:33:48 -0300
From:   Jason Gunthorpe <jgg@...lanox.com>
To:     "Dey, Megha" <megha.dey@...el.com>
Cc:     Dave Jiang <dave.jiang@...el.com>, vkoul@...nel.org,
        maz@...nel.org, bhelgaas@...gle.com, rafael@...nel.org,
        gregkh@...uxfoundation.org, tglx@...utronix.de, hpa@...or.com,
        alex.williamson@...hat.com, jacob.jun.pan@...el.com,
        ashok.raj@...el.com, yi.l.liu@...el.com, baolu.lu@...el.com,
        kevin.tian@...el.com, sanjay.k.kumar@...el.com,
        tony.luck@...el.com, jing.lin@...el.com, dan.j.williams@...el.com,
        kwankhede@...dia.com, eric.auger@...hat.com, parav@...lanox.com,
        dave.hansen@...el.com, netanelg@...lanox.com, shahafs@...lanox.com,
        yan.y.zhao@...ux.intel.com, pbonzini@...hat.com,
        samuel.ortiz@...el.com, mona.hossain@...el.com,
        dmaengine@...r.kernel.org, linux-kernel@...r.kernel.org,
        x86@...nel.org, linux-pci@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH RFC v2 03/18] irq/dev-msi: Create IR-DEV-MSI irq domain

On Wed, Jul 22, 2020 at 10:03:45AM -0700, Dey, Megha wrote:
> Hi Dan,
> 
> On 7/21/2020 9:21 AM, Jason Gunthorpe wrote:
> > On Tue, Jul 21, 2020 at 09:02:35AM -0700, Dave Jiang wrote:
> > > From: Megha Dey <megha.dey@...el.com>
> > > 
> > > When DEV_MSI is enabled, the dev_msi_default_domain is updated to the
> > > base DEV-MSI irq  domain. If interrupt remapping is enabled, we create
> > > a new IR-DEV-MSI irq domain and update the dev_msi_default domain to
> > > the same.
> > > 
> > > For X86, introduce a new irq_alloc_type which will be used by the
> > > interrupt remapping driver.
> > 
> > Why? Shouldn't this by symmetrical with normal MSI? Does MSI do this?
> 
> Since I am introducing the new dev msi domain for the case when IR_REMAP is
> turned on, I have introduced the new type in this patch.
>
> MSI/MSIX have their own irq alloc types which are also only used by the
> intel remapping driver..
>
> > 
> > I would have thought you'd want to switch to this remapping mode as
> > part of vfio or something like current cases.
> 
> Can you let me know what current case you are referring to?

My mistake, I see Intel unconditionally globally enables IR, so this
seems consistent with Intel's MSI

> > > +struct irq_domain *create_remap_dev_msi_irq_domain(struct irq_domain *parent,
> > > +						   const char *name)
> > > +{
> > > +	struct fwnode_handle *fn;
> > > +	struct irq_domain *domain;
> > > +
> > > +	fn = irq_domain_alloc_named_fwnode(name);
> > > +	if (!fn)
> > > +		return NULL;
> > > +
> > > +	domain = msi_create_irq_domain(fn, &dev_msi_ir_domain_info, parent);
> > > +	if (!domain) {
> > > +		pr_warn("failed to initialize irqdomain for IR-DEV-MSI.\n");
> > > +		return ERR_PTR(-ENXIO);
> > > +	}
> > > +
> > > +	irq_domain_update_bus_token(domain, DOMAIN_BUS_PLATFORM_MSI);
> > > +
> > > +	if (!dev_msi_default_domain)
> > > +		dev_msi_default_domain = domain;
> > > +
> > > +	return domain;
> > > +}
> > 
> > What about this code creates a "remap" ? ie why is the function called
> > "create_remap" ?
> 
> Well, this function creates a new domain for the case when IR_REMAP is
> enabled, hence I called it create_remap...

This looks like it just creates a new domain - the thing that makes it
remapping is the caller putting it under the ir_domain - so this code
here in base shouldn't have the word 'remap' in it, this is just
creating a generic domain.

It also kinda looks like create_dev_msi_domain() can just call the
above directly instead of duplicating everything - eg why do we need
two identical dev_msi_ir_controller vs dev_msi_controller just to have
the irq_set_vcpu_affinity difference?

Jason