From: Shawn Anastasio <shawn@anastas.io>
To: Oliver <oohall@gmail.com>
Cc: Sam Bobroff <sbobroff@linux.ibm.com>,
linux-pci@vger.kernel.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
rppt@linux.ibm.com, Paul Mackerras <paulus@samba.org>,
Bjorn Helgaas <bhelgaas@google.com>,
xyjxie@linux.vnet.ibm.com,
linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH v3 1/3] PCI: Introduce pcibios_ignore_alignment_request
Date: Tue, 28 May 2019 00:50:47 -0500 [thread overview]
Message-ID: <a5966412-e853-3f74-2459-d68597629fc1@anastas.io> (raw)
In-Reply-To: <CAOSf1CEFfbmwfvmdqT1xdt8SFb=tYdYXLfXeyZ8=iRnhg4a3Pg@mail.gmail.com>
On 5/28/19 12:36 AM, Oliver wrote:
> On Tue, May 28, 2019 at 2:03 PM Shawn Anastasio <shawn@anastas.io> wrote:
>>
>> Introduce a new pcibios function pcibios_ignore_alignment_request
>> which allows the PCI core to defer to platform-specific code to
>> determine whether or not to ignore alignment requests for PCI resources.
>>
>> The existing behavior is to simply ignore alignment requests when
>> PCI_PROBE_ONLY is set. This is behavior is maintained by the
>> default implementation of pcibios_ignore_alignment_request.
>>
>> Signed-off-by: Shawn Anastasio <shawn@anastas.io>
>> ---
>> drivers/pci/pci.c | 9 +++++++--
>> include/linux/pci.h | 1 +
>> 2 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 8abc843b1615..8207a09085d1 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -5882,6 +5882,11 @@ resource_size_t __weak pcibios_default_alignment(void)
>> return 0;
>> }
>>
>> +int __weak pcibios_ignore_alignment_request(void)
>> +{
>> + return pci_has_flag(PCI_PROBE_ONLY);
>> +}
>> +
>> #define RESOURCE_ALIGNMENT_PARAM_SIZE COMMAND_LINE_SIZE
>> static char resource_alignment_param[RESOURCE_ALIGNMENT_PARAM_SIZE] = {0};
>> static DEFINE_SPINLOCK(resource_alignment_lock);
>> @@ -5906,9 +5911,9 @@ static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
>> p = resource_alignment_param;
>> if (!*p && !align)
>> goto out;
>> - if (pci_has_flag(PCI_PROBE_ONLY)) {
>> + if (pcibios_ignore_alignment_request()) {
>> align = 0;
>> - pr_info_once("PCI: Ignoring requested alignments (PCI_PROBE_ONLY)\n");
>> + pr_info_once("PCI: Ignoring requested alignments\n");
>> goto out;
>> }
>
> I think the logic here is questionable to begin with. If the user has
> explicitly requested re-aligning a resource via the command line then
> we should probably do it even if PCI_PROBE_ONLY is set. When it breaks
> they get to keep the pieces.
>
> That said, the real issue here is that PCI_PROBE_ONLY probably
> shouldn't be set under qemu/kvm. Under the other hypervisor (PowerVM)
> hotplugged devices are configured by firmware before it's passed to
> the guest and we need to keep the FW assignments otherwise things
> break. QEMU however doesn't do any BAR assignments and relies on that
> being handled by the guest. At boot time this is done by SLOF, but
> Linux only keeps SLOF around until it's extracted the device-tree.
> Once that's done SLOF gets blown away and the kernel needs to do it's
> own BAR assignments. I'm guessing there's a hack in there to make it
> work today, but it's a little surprising that it works at all...
Interesting, I wasn't aware that hotplugged devices are configured
by the hypervisor on PowerVM. That at least means that this patch is
wrong as-is since it won't handle that properly. Definitely seems like
there will need to be different behavior here depending on the hypervisor.
That being said, wouldn't PCI_PROBE_ONLY still be set on pseries/kvm
(at least for initial boot) to observe SLOF's original BAR assignments?
Perhaps it should be un-set after initial PCI init?
>
> IIRC Sam Bobroff was looking at hotplug under pseries recently so he
> might have something to add. He's sick at the moment, but I'll ask him
> to take a look at this once he's back among the living
Good to know. I'll await his comments before continuing here.
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 4a5a84d7bdd4..47471dcdbaf9 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -1990,6 +1990,7 @@ static inline void pcibios_penalize_isa_irq(int irq, int active) {}
>> int pcibios_alloc_irq(struct pci_dev *dev);
>> void pcibios_free_irq(struct pci_dev *dev);
>> resource_size_t pcibios_default_alignment(void);
>> +int pcibios_ignore_alignment_request(void);
>>
>> #ifdef CONFIG_HIBERNATE_CALLBACKS
>> extern struct dev_pm_ops pcibios_pm_ops;
>> --
>> 2.20.1
>>
next prev parent reply other threads:[~2019-05-28 5:50 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-28 4:03 [PATCH v3 0/3] Allow custom PCI resource alignment on pseries Shawn Anastasio
2019-05-28 4:03 ` [PATCH v3 1/3] PCI: Introduce pcibios_ignore_alignment_request Shawn Anastasio
2019-05-28 5:36 ` Oliver
2019-05-28 5:50 ` Shawn Anastasio [this message]
2019-05-28 6:27 ` Alexey Kardashevskiy
2019-05-28 7:39 ` Shawn Anastasio
2019-05-30 3:39 ` Alexey Kardashevskiy
2019-05-30 22:49 ` Shawn Anastasio
2019-05-31 3:56 ` Alexey Kardashevskiy
2019-06-03 2:23 ` Shawn Anastasio
2019-06-03 5:02 ` Alexey Kardashevskiy
2019-06-03 8:35 ` Alexey Kardashevskiy
2019-06-03 9:12 ` Shawn Anastasio
2019-05-29 14:00 ` Bjorn Helgaas
2019-05-30 6:55 ` [EXTERNAL] " Sam Bobroff
2019-05-30 22:33 ` Shawn Anastasio
2019-05-28 4:03 ` [PATCH v3 2/3] powerpc/64: Enable pcibios_after_init hook on ppc64 Shawn Anastasio
2019-05-28 4:03 ` [PATCH v3 3/3] powerpc/pseries: Allow user-specified PCI resource alignment after init Shawn Anastasio
2019-05-29 14:02 ` Bjorn Helgaas
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=a5966412-e853-3f74-2459-d68597629fc1@anastas.io \
--to=shawn@anastas.io \
--cc=bhelgaas@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=oohall@gmail.com \
--cc=paulus@samba.org \
--cc=rppt@linux.ibm.com \
--cc=sbobroff@linux.ibm.com \
--cc=xyjxie@linux.vnet.ibm.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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).