Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 23 Jun 2022 11:03:02 -0700
From:   Dan Williams <dan.j.williams@intel.com>
To:     Ira Weiny <ira.weiny@intel.com>,
        Jonathan Cameron <Jonathan.Cameron@huawei.com>
CC:     Dan Williams <dan.j.williams@intel.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Alison Schofield <alison.schofield@intel.com>,
        "Vishal Verma" <vishal.l.verma@intel.com>,
        Dave Jiang <dave.jiang@intel.com>,
        "Ben Widawsky" <bwidawsk@kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-cxl@vger.kernel.org>,
        <linux-pci@vger.kernel.org>
Subject: Re: [PATCH V11 3/8] PCI: Create PCI library functions in support of
 DOE mailboxes.
Message-ID: <62b4aad6a2e71_3c3ff0294aa@dwillia2-xfh.notmuch>
References: <20220610202259.3544623-1-ira.weiny@intel.com>
 <20220610202259.3544623-4-ira.weiny@intel.com>
 <62ad06a643ddf_844b1294f@dwillia2-xfh.notmuch>
 <20220620112328.00004f63@Huawei.com>
 <YrOea1PQbSA09A6r@iweiny-desk3>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <YrOea1PQbSA09A6r@iweiny-desk3>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?AdvUhLXhVii81sSFcoC21r1tB8p1aJP6Z6JLQjuXoEDkj12gFhPPJIMwpMRO?=
 =?us-ascii?Q?cWrcAi2yB1yBKJrosD4IDmf1w+EyF9S7UtiP0VudDrHbg4VSm6LKdcfyOZgS?=
 =?us-ascii?Q?p7kLYwYuFSoEJFuPpf7kwUoRVK+1klisuyLVAbag4G1xhtT2059RnXIHy91f?=
 =?us-ascii?Q?xTfIEbg/W3SiW22rpgCX5o0kncTRG71xAi7WUbdfYszWlOMbxappHX66t7IO?=
 =?us-ascii?Q?FjwhK7BAlKiQq1uOHvw8IfE2VCUqoqfzqrrNHAfBcZQBGnInX0DX+eHSp7Wq?=
 =?us-ascii?Q?S/0xiebvP2MGGk8PQ5u0B7CWmlNKNOuUQXQQPsSUt+02j/q3Ga0TUlDUF2Up?=
 =?us-ascii?Q?bAh9Nh6ITfYSShIqIwWsIqHySaqd/AMj++KpflBmHBLPahss2vzN7Lzg1nhi?=
 =?us-ascii?Q?0WfRslDklyKjHADZJCM/tmEUaX0fJUc2Brr9aCyI9y+oxoicjyyq/fPkwJ27?=
 =?us-ascii?Q?yApFbK4mTlyC2o4ugMYWm/ePMTEoL9iNC53p9wtw51durnSJMCKGW9iXQiaB?=
 =?us-ascii?Q?n3RDt1Ptns/jA75meY0ft8ejT6/ICZLMOyvT2A47+B7JduVoE7Hqn75TRR1C?=
 =?us-ascii?Q?o9MRz1S0ZEwfIuxGI2cht00lLmmd2I/+20DgFvL7UEdsaXcCzljDYrdH3Tkn?=
 =?us-ascii?Q?Q+dUxGkuDUSrQr8RpFpZ2WnTiAJ35gUHHAlAXOKTIExhcsJvXqlQ7ST8hKtE?=
 =?us-ascii?Q?NS7Uq681BdDLQf+6LBpZ1k2uqpP/D3rSfKs5zR6Jlj01Gt0Ge5n3xt41epJT?=
 =?us-ascii?Q?DKd3440Vpi4McikhYJy+JlcltHhQathbbiLi/ogHuIK6CS6SuMHLR+7+EsaX?=
 =?us-ascii?Q?b5Imi4X0Xo+G7C+AAO/UuOiDaMsLFIJ99+H3q3dPVbFdUI3IDdGnI8XtccOH?=
 =?us-ascii?Q?8a3w3C5x6LFTnIbhC4WuXl3uTurxScwF7mZs/AmkwgbaDqx1XOZKIyDIduhU?=
 =?us-ascii?Q?5jWgmZKqnVaf5vKK+FiGSPdgyaWXLmeAeSMKmrGa4TMkS/T7/T/sPXOtDSaq?=
 =?us-ascii?Q?05xfgZB+oF5ubW8hkcBoDIHbcW3w4zBuBH1Clj2SfaMMhxy2Y3TmtcnUTSbo?=
 =?us-ascii?Q?6TgGVZlivvaEkm1RB+dXZBAS4qJz4+soFPzThtJzK3NQ8LRvqz/7G4D6WonB?=
 =?us-ascii?Q?gH6BvUEinduDrtX8cFRWk2WBRCsOgCz9lJRbCIBgdUVHqeP0C+wOlvEKRFB8?=
 =?us-ascii?Q?TzRVoiuhT0cJ1agKLflgsPxcTNigvkCW7QhKvJQibQEnNZpQCnnp5UzEkDAG?=
 =?us-ascii?Q?KL7V8hxt1WnuogETOKNhLK5rFoUjzby1MgzEi7fjqIs3EgLaL9obx4QErDpX?=
 =?us-ascii?Q?R4YH+c3FBt9hqsRAyEKVvxiMKQqoHrmVUmG23EFod5ht1cUKXWuuJaoFU+pd?=
 =?us-ascii?Q?E5ILpHM9Anitwrh47w5Vm549q+Lb7Ug0+PqUQYoVWR2e1GL+1ADzUw+/0Rew?=
 =?us-ascii?Q?+DETY4zi/NIpD9QRgs8za4cwgXXrPh6robb9QTZ/Qb3XkAyQ8DaNIkt3rtge?=
 =?us-ascii?Q?BebJ70U/9iW1aXc2R4bS4rlPPfk/pCvTH6eRCxA9BXJYQqfJtyjO8el/8NOG?=
 =?us-ascii?Q?cDLgQBxLnWNoSuoYb9kZNFiKHosPrMPc9RsgBKZ2rOU6qZiyqnb4uQSrqF2f?=
 =?us-ascii?Q?mk47X1eXbUrgalJEaVpgkgNL4CoqU+5lfxpucEdUDgzEiY2dJPZXTg6fUCuD?=
 =?us-ascii?Q?G2Z9QtSQ3n/XrVtmuSzzIuVZo1yrP/Seb0r1lAuYmNhf0eAQvCnl/By+5GZ2?=
 =?us-ascii?Q?P2xk+KZUEcfNczggcOFa90zFnzuWPW0=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 3156e646-02cb-41d2-7ab9-08da55429ee1
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1101MB2126.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Jun 2022 18:03:04.9844
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 58ufXz5yX0xQT5AcfANyfmVxnKUInoNZIVvW1Qp5xI4fw1gfphp3gdY38LQ6oKlrlf0e3z+KU4wQFKsB/ACszO/MsOOpMAe6Z0ItRQUcPX8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB4874
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Ira Weiny wrote:
> On Mon, Jun 20, 2022 at 11:23:28AM +0100, Jonathan Cameron wrote:
> > On Fri, 17 Jun 2022 15:56:38 -0700
> > Dan Williams <dan.j.williams@intel.com> wrote:
> > 
> 
> [snip]
> 
> > > > +static int pci_doe_enable_irq(struct pci_doe_mb *doe_mb, unsigned int irq)
> > > > +{
> > > > +	struct pci_dev *pdev = doe_mb->pdev;
> > > > +	int offset = doe_mb->cap_offset;
> > > > +	int rc;
> > > > +
> > > > +	/*
> > > > +	 * Enabling bus mastering is required for MSI/MSIx.  It is safe to call
> > > > +	 * this multiple times and thus is called here to ensure that mastering
> > > > +	 * is enabled even if the driver has done so.
> > > > +	 */  
> > > 
> > > It is required for MSI/MSIx to work, yes, but if the caller that created
> > > the doe object has not set it yet that's its prerogative.
> > 
> > We went around this in one of the earlier threads, and I think consensus
> > was that it made sense to have this in core code.
> 
> Yes we did.  This is where Bjorn asked for this to be done I thought.
> 
> I verified that pci_set_master() is fine to call more than one time.  If the
> caller asks for irq support I think it is fine to do this here to ensure that
> support is enabled.

Its moot now that irq support is being pushed out to a follow-on
change, but I think its unfortunate that this makes it difficult to
audit when and where a device will be enabled to send cycles. If at all
possible I think it makes sense to keep this the responsibility of the
driver for the pci_dev. Similar to how pcie_port_device_register() does
this on behalf of the services it registers rather than each service
doing this itself.