dmaengine Archive on lore.kernel.org
 help / color / Atom feed
From: "Dey, Megha" <megha.dey@intel.com>
To: Thomas Gleixner <tglx@linutronix.de>,
	"Jiang, Dave" <dave.jiang@intel.com>,
	"vkoul@kernel.org" <vkoul@kernel.org>,
	"maz@kernel.org" <maz@kernel.org>,
	"bhelgaas@google.com" <bhelgaas@google.com>,
	"rafael@kernel.org" <rafael@kernel.org>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"hpa@zytor.com" <hpa@zytor.com>,
	"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	"Pan, Jacob jun" <jacob.jun.pan@intel.com>,
	"Raj, Ashok" <ashok.raj@intel.com>,
	"jgg@mellanox.com" <jgg@mellanox.com>,
	"Liu, Yi L" <yi.l.liu@intel.com>,
	"Lu, Baolu" <baolu.lu@intel.com>,
	"Tian, Kevin" <kevin.tian@intel.com>,
	"Kumar, Sanjay K" <sanjay.k.kumar@intel.com>,
	"Luck, Tony" <tony.luck@intel.com>,
	"Lin, Jing" <jing.lin@intel.com>,
	"Williams, Dan J" <dan.j.williams@intel.com>,
	"kwankhede@nvidia.com" <kwankhede@nvidia.com>,
	"eric.auger@redhat.com" <eric.auger@redhat.com>,
	"parav@mellanox.com" <parav@mellanox.com>,
	"jgg@mellanox.com" <jgg@mellanox.com>,
	"rafael@kernel.org" <rafael@kernel.org>,
	"Hansen, Dave" <dave.hansen@intel.com>,
	"netanelg@mellanox.com" <netanelg@mellanox.com>,
	"shahafs@mellanox.com" <shahafs@mellanox.com>,
	"yan.y.zhao@linux.intel.com" <yan.y.zhao@linux.intel.com>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	"Ortiz, Samuel" <samuel.ortiz@intel.com>,
	"Hossain, Mona" <mona.hossain@intel.com>
Cc: "dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"x86@kernel.org" <x86@kernel.org>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: RE: [PATCH RFC v2 03/18] irq/dev-msi: Create IR-DEV-MSI irq domain
Date: Wed, 5 Aug 2020 19:02:35 +0000
Message-ID: <ec81670539674beea58c9a6c4104b26b@intel.com> (raw)
In-Reply-To: <87lfjbz3cs.fsf@nanos.tec.linutronix.de>

Hi Thomas,

> -----Original Message-----
> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: Wednesday, July 22, 2020 1:45 PM
> To: Jiang, Dave <dave.jiang@intel.com>; vkoul@kernel.org; Dey, Megha
> <megha.dey@intel.com>; maz@kernel.org; bhelgaas@google.com;
> rafael@kernel.org; gregkh@linuxfoundation.org; hpa@zytor.com;
> alex.williamson@redhat.com; Pan, Jacob jun <jacob.jun.pan@intel.com>; Raj,
> Ashok <ashok.raj@intel.com>; jgg@mellanox.com; Liu, Yi L <yi.l.liu@intel.com>;
> Lu, Baolu <baolu.lu@intel.com>; Tian, Kevin <kevin.tian@intel.com>; Kumar,
> Sanjay K <sanjay.k.kumar@intel.com>; Luck, Tony <tony.luck@intel.com>; Lin,
> Jing <jing.lin@intel.com>; Williams, Dan J <dan.j.williams@intel.com>;
> kwankhede@nvidia.com; eric.auger@redhat.com; parav@mellanox.com;
> jgg@mellanox.com; rafael@kernel.org; Hansen, Dave
> <dave.hansen@intel.com>; netanelg@mellanox.com; shahafs@mellanox.com;
> yan.y.zhao@linux.intel.com; pbonzini@redhat.com; Ortiz, Samuel
> <samuel.ortiz@intel.com>; Hossain, Mona <mona.hossain@intel.com>
> Cc: dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org;
> x86@kernel.org; linux-pci@vger.kernel.org; kvm@vger.kernel.org
> Subject: Re: [PATCH RFC v2 03/18] irq/dev-msi: Create IR-DEV-MSI irq domain
> 
> Dave Jiang <dave.jiang@intel.com> writes:
> > From: Megha Dey <megha.dey@intel.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
> 
> s/we//

ok
> 
> > 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.
> >
> > Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Megha Dey <megha.dey@intel.com>
> > Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> > ---
> >  arch/x86/include/asm/hw_irq.h       |    1 +
> >  arch/x86/kernel/apic/msi.c          |   12 ++++++
> >  drivers/base/dev-msi.c              |   66 +++++++++++++++++++++++++++++++----
> >  drivers/iommu/intel/irq_remapping.c |   11 +++++-
> >  include/linux/intel-iommu.h         |    1 +
> >  include/linux/irqdomain.h           |   11 ++++++
> >  include/linux/msi.h                 |    3 ++
> 
> Why is this mixing generic code, x86 core code and intel specific driver code?
> This is new functionality so:
> 
>       1) Provide the infrastructure
>       2) Add support to architecture specific parts
>       3) Enable it

Ok, I will try to adhere to the layering next time around..
> 
> > +
> > +#ifdef CONFIG_DEV_MSI
> > +int dev_msi_prepare(struct irq_domain *domain, struct device *dev,
> > +			   int nvec, msi_alloc_info_t *arg) {
> > +	memset(arg, 0, sizeof(*arg));
> > +
> > +	arg->type = X86_IRQ_ALLOC_TYPE_DEV_MSI;
> > +
> > +	return 0;
> > +}
> > +#endif
> 
> What is this? Tons of new lines for taking up more space and not a single
> comment.

Hmm, I will add a comment..
> 
> > -static int dev_msi_prepare(struct irq_domain *domain, struct device
> > *dev,
> > +int __weak dev_msi_prepare(struct irq_domain *domain, struct device
> > +*dev,
> >  			   int nvec, msi_alloc_info_t *arg)  {
> >  	memset(arg, 0, sizeof(*arg));
> 
> Oh well. So every architecure which needs to override this and I assume all
> which are eventually going to support it need to do the memset() in their
> override.
> 
>        memset(arg,,,);
>        arch_dev_msi_prepare();
> 
> 
Per you suggestion, I have introduced arch_dev_msi_prepare which returns 0 by default unless
overridden by arch code in the next patch set.

> > -	dev_msi_default_domain = msi_create_irq_domain(fn,
> &dev_msi_domain_info, parent);
> > +	/*
> > +	 * This initcall may come after remap code is initialized. Ensure that
> > +	 * dev_msi_default domain is updated correctly.
> 
> What? No, this is a disgusting hack. Get your ordering straight, that's not rocket
> science.
> 

Hmm yeah, actually I realized we don't really need to have 2 new IRQ domains for dev-msi 
(with and without interrupt remapping enabled). Hence all this will go away in the next round
of patches.

> > +#ifdef CONFIG_IRQ_REMAP
> 
> IRQ_REMAP is x86 specific. Is this file x86 only or intended to be for general use?
> If it's x86 only, then this should be clearly documented. If not, then these
> x86'isms have no place here.

True, I will take care of this in the next patch set.
> 
> > +struct irq_domain *create_remap_dev_msi_irq_domain(struct irq_domain
> *parent,
> > +						   const char *name)
> 
> So we have msi_create_irq_domain() and this is about dev_msi, right? So can
> you please stick with a consistent naming scheme?

sure
> 
> > +{
> > +	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;
> 
> Can this be called several times? If so, then this lacks a comment. If not, then
> this condition is useless.

Hmm this will go way in the next patch set, thank you for your input!
> 
> Thanks,
> 
>         tglx

  reply index

Thread overview: 97+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-21 16:02 [PATCH RFC v2 00/18] Add VFIO mediated device support and DEV-MSI support for the idxd driver Dave Jiang
2020-07-21 16:02 ` [PATCH RFC v2 01/18] platform-msi: Introduce platform_msi_ops Dave Jiang
2020-07-21 16:02 ` [PATCH RFC v2 02/18] irq/dev-msi: Add support for a new DEV_MSI irq domain Dave Jiang
2020-07-21 16:13   ` Jason Gunthorpe
2020-07-22 16:50     ` Dey, Megha
2020-07-22 18:52   ` Marc Zyngier
2020-07-22 19:59     ` Jason Gunthorpe
2020-07-23  8:51       ` Marc Zyngier
2020-07-24  0:16         ` Jason Gunthorpe
2020-07-24  0:36           ` Thomas Gleixner
2020-08-05 19:18       ` Dey, Megha
2020-08-05 22:15         ` Jason Gunthorpe
2020-08-05 22:36           ` Dey, Megha
2020-08-05 22:53             ` Jason Gunthorpe
2020-08-06  0:13               ` Dey, Megha
2020-08-06  0:19                 ` Jason Gunthorpe
2020-08-06  0:32                   ` Dey, Megha
2020-08-06  0:46                     ` Jason Gunthorpe
2020-08-06 17:10                     ` Thomas Gleixner
2020-08-06 17:58                       ` Dey, Megha
2020-08-06 20:21                         ` Thomas Gleixner
2020-08-06 22:27                           ` Dey, Megha
2020-08-07  8:48                             ` Thomas Gleixner
2020-08-07 12:06                           ` Jason Gunthorpe
2020-08-07 12:38                             ` gregkh
2020-08-07 13:34                               ` Jason Gunthorpe
2020-08-07 16:47                                 ` Thomas Gleixner
2020-08-07 17:54                                   ` Dey, Megha
2020-08-07 18:39                                     ` Jason Gunthorpe
2020-08-07 20:31                                       ` Dey, Megha
2020-08-08 19:47                                     ` Thomas Gleixner
2020-08-10 21:46                                       ` Thomas Gleixner
2020-08-11  9:53                                         ` Thomas Gleixner
2020-08-11 18:46                                           ` Dey, Megha
2020-08-11 21:25                                             ` Thomas Gleixner
2020-08-11 18:39                                       ` Dey, Megha
2020-08-11 22:39                                         ` Thomas Gleixner
2020-08-07 15:22                             ` Thomas Gleixner
2020-08-05 18:55     ` Dey, Megha
2020-07-21 16:02 ` [PATCH RFC v2 03/18] irq/dev-msi: Create IR-DEV-MSI " Dave Jiang
2020-07-21 16:21   ` Jason Gunthorpe
2020-07-22 17:03     ` Dey, Megha
2020-07-22 17:33       ` Jason Gunthorpe
2020-07-22 20:44   ` Thomas Gleixner
2020-08-05 19:02     ` Dey, Megha [this message]
2020-07-21 16:02 ` [PATCH RFC v2 04/18] irq/dev-msi: Introduce APIs to allocate/free dev-msi interrupts Dave Jiang
2020-07-21 16:25   ` Jason Gunthorpe
2020-07-22 17:05     ` Dey, Megha
2020-07-22 17:35       ` Jason Gunthorpe
2020-08-05 20:19         ` Dey, Megha
2020-07-21 16:02 ` [PATCH RFC v2 05/18] dmaengine: idxd: add support for readonly config devices Dave Jiang
2020-07-21 16:02 ` [PATCH RFC v2 06/18] dmaengine: idxd: add interrupt handle request support Dave Jiang
2020-07-21 16:03 ` [PATCH RFC v2 07/18] dmaengine: idxd: add DEV-MSI support in base driver Dave Jiang
2020-07-21 16:03 ` [PATCH RFC v2 08/18] dmaengine: idxd: add device support functions in prep for mdev Dave Jiang
2020-07-21 16:03 ` [PATCH RFC v2 09/18] dmaengine: idxd: add basic mdev registration and helper functions Dave Jiang
2020-07-21 16:03 ` [PATCH RFC v2 10/18] dmaengine: idxd: add emulation rw routines Dave Jiang
2020-07-21 16:03 ` [PATCH RFC v2 11/18] dmaengine: idxd: prep for virtual device commands Dave Jiang
2020-07-21 16:03 ` [PATCH RFC v2 12/18] dmaengine: idxd: virtual device commands emulation Dave Jiang
2020-07-21 16:03 ` [PATCH RFC v2 13/18] dmaengine: idxd: ims setup for the vdcm Dave Jiang
2020-07-21 16:03 ` [PATCH RFC v2 14/18] dmaengine: idxd: add mdev type as a new wq type Dave Jiang
2020-07-21 16:03 ` [PATCH RFC v2 15/18] dmaengine: idxd: add dedicated wq mdev type Dave Jiang
2020-07-21 16:04 ` [PATCH RFC v2 16/18] dmaengine: idxd: add new wq state for mdev Dave Jiang
2020-07-21 16:04 ` [PATCH RFC v2 17/18] dmaengine: idxd: add error notification from host driver to mediated device Dave Jiang
2020-07-21 16:04 ` [PATCH RFC v2 18/18] dmaengine: idxd: add ABI documentation for mediated device support Dave Jiang
2020-07-21 16:28 ` [PATCH RFC v2 00/18] Add VFIO mediated device support and DEV-MSI support for the idxd driver Greg KH
2020-07-21 17:17   ` Dave Jiang
2020-07-21 21:35   ` Dan Williams
2020-07-21 16:45 ` Jason Gunthorpe
2020-07-21 18:00   ` Dave Jiang
2020-07-22 17:31     ` Dey, Megha
2020-07-22 18:16       ` Jason Gunthorpe
2020-07-21 23:54   ` Tian, Kevin
2020-07-24  0:19     ` Jason Gunthorpe
2020-08-06  1:22       ` Alex Williamson
2020-08-07 12:19         ` Jason Gunthorpe
2020-08-10  7:32           ` Tian, Kevin
2020-08-11 17:00             ` Alex Williamson
2020-08-12  1:58               ` Tian, Kevin
2020-08-12  2:36                 ` Alex Williamson
2020-08-12  3:35                   ` Tian, Kevin
2020-08-12  3:28             ` Jason Wang
2020-08-12  4:05               ` Tian, Kevin
2020-08-13  4:33                 ` Jason Wang
2020-08-13  5:26                   ` Tian, Kevin
2020-08-13  6:01                     ` Jason Wang
2020-08-14 13:23                       ` Jason Gunthorpe
2020-08-17  2:24                         ` Tian, Kevin
2020-08-14 13:35             ` Jason Gunthorpe
2020-08-17  2:12               ` Tian, Kevin
2020-08-18  0:43                 ` Jason Gunthorpe
2020-08-18  1:09                   ` Tian, Kevin
2020-08-18 11:50                     ` Jason Gunthorpe
2020-08-18 16:27                       ` Paolo Bonzini
2020-08-18 16:49                         ` Jason Gunthorpe
2020-08-18 17:05                           ` Paolo Bonzini
2020-08-18 17:18                             ` Jason Gunthorpe
2020-08-19  7:29                       ` Tian, Kevin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ec81670539674beea58c9a6c4104b26b@intel.com \
    --to=megha.dey@intel.com \
    --cc=alex.williamson@redhat.com \
    --cc=ashok.raj@intel.com \
    --cc=baolu.lu@intel.com \
    --cc=bhelgaas@google.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=eric.auger@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hpa@zytor.com \
    --cc=jacob.jun.pan@intel.com \
    --cc=jgg@mellanox.com \
    --cc=jing.lin@intel.com \
    --cc=kevin.tian@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=kwankhede@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=mona.hossain@intel.com \
    --cc=netanelg@mellanox.com \
    --cc=parav@mellanox.com \
    --cc=pbonzini@redhat.com \
    --cc=rafael@kernel.org \
    --cc=samuel.ortiz@intel.com \
    --cc=sanjay.k.kumar@intel.com \
    --cc=shahafs@mellanox.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=vkoul@kernel.org \
    --cc=x86@kernel.org \
    --cc=yan.y.zhao@linux.intel.com \
    --cc=yi.l.liu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

dmaengine Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dmaengine/0 dmaengine/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dmaengine dmaengine/ https://lore.kernel.org/dmaengine \
		dmaengine@vger.kernel.org
	public-inbox-index dmaengine

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.dmaengine


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git