| 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
| ||
|
Message-ID: <c7877f72-97e0-ac48-06c3-8e3ecec87cd5@ti.com>
Date: Thu, 5 Dec 2019 16:52:02 +0530
From: Kishon Vijay Abraham I <kishon@...com>
To: Vidya Sagar <vidyas@...dia.com>, <jingoohan1@...il.com>,
<gustavo.pimentel@...opsys.com>, <lorenzo.pieralisi@....com>,
<andrew.murray@....com>, <bhelgaas@...gle.com>,
<thierry.reding@...il.com>
CC: <Jisheng.Zhang@...aptics.com>, <jonathanh@...dia.com>,
<linux-pci@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<kthota@...dia.com>, <mmaddireddy@...dia.com>, <sagar.tv@...il.com>
Subject: Re: [PATCH 4/4] PCI: pci-epf-test: Add support to defer core
initialization
Hi,
On 01/12/19 7:59 pm, Vidya Sagar wrote:
> On 11/27/2019 2:50 PM, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On 13/11/19 2:38 PM, Vidya Sagar wrote:
>>> Add support to defer core initialization and to receive a notifier
>>> when core is ready to accommodate platforms where core is not for
>>> initialization untile reference clock from host is available.
>>>
>>> Signed-off-by: Vidya Sagar <vidyas@...dia.com>
>>> ---
>>> drivers/pci/endpoint/functions/pci-epf-test.c | 114 ++++++++++++------
>>> 1 file changed, 77 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c
>>> b/drivers/pci/endpoint/functions/pci-epf-test.c
>>> index bddff15052cc..068024fab544 100644
>>> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
>>> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
>>> @@ -360,18 +360,6 @@ static void pci_epf_test_cmd_handler(struct
>>> work_struct *work)
>>> msecs_to_jiffies(1));
>>> }
>>> -static int pci_epf_test_notifier(struct notifier_block *nb, unsigned
>>> long val,
>>> - void *data)
>>> -{
>>> - struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
>>> - struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>> -
>>> - queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
>>> - msecs_to_jiffies(1));
>>> -
>>> - return NOTIFY_OK;
>>> -}
>>> -
>>> static void pci_epf_test_unbind(struct pci_epf *epf)
>>> {
>>> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>> @@ -428,6 +416,78 @@ static int pci_epf_test_set_bar(struct pci_epf
>>> *epf)
>>> return 0;
>>> }
>>> +static int pci_epf_test_core_init(struct pci_epf *epf)
>>> +{
>>> + struct pci_epf_header *header = epf->header;
>>> + const struct pci_epc_features *epc_features;
>>> + struct pci_epc *epc = epf->epc;
>>> + struct device *dev = &epf->dev;
>>> + bool msix_capable = false;
>>> + bool msi_capable = true;
>>> + int ret;
>>> +
>>> + epc_features = pci_epc_get_features(epc, epf->func_no);
>>> + if (epc_features) {
>>> + msix_capable = epc_features->msix_capable;
>>> + msi_capable = epc_features->msi_capable;
>>> + }
>>> +
>>> + ret = pci_epc_write_header(epc, epf->func_no, header);
>>> + if (ret) {
>>> + dev_err(dev, "Configuration header write failed\n");
>>> + return ret;
>>> + }
>>> +
>>> + ret = pci_epf_test_set_bar(epf);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (msi_capable) {
>>> + ret = pci_epc_set_msi(epc, epf->func_no, epf->msi_interrupts);
>>> + if (ret) {
>>> + dev_err(dev, "MSI configuration failed\n");
>>> + return ret;
>>> + }
>>> + }
>>> +
>>> + if (msix_capable) {
>>> + ret = pci_epc_set_msix(epc, epf->func_no,
>>> epf->msix_interrupts);
>>> + if (ret) {
>>> + dev_err(dev, "MSI-X configuration failed\n");
>>> + return ret;
>>> + }
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int pci_epf_test_notifier(struct notifier_block *nb, unsigned
>>> long val,
>>> + void *data)
>>> +{
>>> + struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
>>> + struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>> + int ret;
>>> +
>>> + switch (val) {
>>> + case CORE_INIT:
>>> + ret = pci_epf_test_core_init(epf);
>>> + if (ret)
>>> + return NOTIFY_BAD;
>>> + break;
>>> +
>>> + case LINK_UP:
>>> + queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
>>> + msecs_to_jiffies(1));
>>> + break;
>>> +
>>> + default:
>>> + dev_err(&epf->dev, "Invalid EPF test notifier event\n");
>>> + return NOTIFY_BAD;
>>> + }
>>> +
>>> + return NOTIFY_OK;
>>> +}
>>> +
>>> static int pci_epf_test_alloc_space(struct pci_epf *epf)
>>> {
>>> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>> @@ -496,12 +556,11 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>>> {
>>> int ret;
>>> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>> - struct pci_epf_header *header = epf->header;
>>> const struct pci_epc_features *epc_features;
>>> enum pci_barno test_reg_bar = BAR_0;
>>> struct pci_epc *epc = epf->epc;
>>> - struct device *dev = &epf->dev;
>>> bool linkup_notifier = false;
>>> + bool skip_core_init = false;
>>> bool msix_capable = false;
>>> bool msi_capable = true;
>>> @@ -511,6 +570,7 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>>> epc_features = pci_epc_get_features(epc, epf->func_no);
>>> if (epc_features) {
>>> linkup_notifier = epc_features->linkup_notifier;
>>> + skip_core_init = epc_features->skip_core_init;
>>> msix_capable = epc_features->msix_capable;
>>> msi_capable = epc_features->msi_capable;
>>
>> Are these used anywhere in this function?
> Nope. I'll remove them.
>
>>> test_reg_bar = pci_epc_get_first_free_bar(epc_features);
>>> @@ -520,34 +580,14 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>>> epf_test->test_reg_bar = test_reg_bar;
>>> epf_test->epc_features = epc_features;
>>> - ret = pci_epc_write_header(epc, epf->func_no, header);
>>> - if (ret) {
>>> - dev_err(dev, "Configuration header write failed\n");
>>> - return ret;
>>> - }
>>> -
>>> ret = pci_epf_test_alloc_space(epf);
>>> if (ret)
>>> return ret;
>>> - ret = pci_epf_test_set_bar(epf);
>>> - if (ret)
>>> - return ret;
>>> -
>>> - if (msi_capable) {
>>> - ret = pci_epc_set_msi(epc, epf->func_no, epf->msi_interrupts);
>>> - if (ret) {
>>> - dev_err(dev, "MSI configuration failed\n");
>>> - return ret;
>>> - }
>>> - }
>>> -
>>> - if (msix_capable) {
>>> - ret = pci_epc_set_msix(epc, epf->func_no,
>>> epf->msix_interrupts);
>>> - if (ret) {
>>> - dev_err(dev, "MSI-X configuration failed\n");
>>> + if (!skip_core_init) {
>>> + ret = pci_epf_test_core_init(epf);
>>> + if (ret)
>>> return ret;
>>> - }
>>> }
>>> if (linkup_notifier) {
>>
>> This could as well be moved to pci_epf_test_core_init().
> Yes, but I would like to keep only the code that touches hardware in
> pci_epf_test_core_init()
> to minimize the time it takes to execute it. Is there any strong reason
> to move it? if not,
> I would prefer to leave it here in this function itself.
There is no point in scheduling a work to check for commands from host
when the EP itself is not initialized.
Thanks
Kishon
Powered by blists - more mailing lists