lkml.org 
[lkml]   [2020]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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@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
    > > > 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

    \
     
     \ /
      Last update: 2020-07-22 19:34    [W:4.308 / U:0.344 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site