[RESEND PATCH 0/3] Allow custom PCI resource alignment on pseries

[RESEND PATCH 0/3] Allow custom PCI resource alignment on pseries

[Shawn Anastasio]

Hello all,

This patch set implements support for user-specified PCI resource
alignment on the pseries platform for hotplugged PCI devices.
Currently on pseries, PCI resource alignments specified with the
pci=resource_alignment commandline argument are ignored, since
the firmware is in charge of managing the PCI resources. In the
case of hotplugged devices, though, the kernel is in charge of 
configuring the resources and should obey alignment requirements.

The current behavior of ignoring the alignment for hotplugged devices
results in sub-page BARs landing between page boundaries and
becoming un-mappable from userspace via the VFIO framework.
This issue was observed on a pseries KVM guest with hotplugged
ivshmem devices.
 
With these changes, users can specify an appropriate
pci=resource_alignment argument on boot for devices they wish to use 
with VFIO.

In the future, this could be extended to provide page-aligned
resources by default for hotplugged devices, similar to what is done
on powernv by commit 382746376993 ("powerpc/powernv: Override
pcibios_default_alignment() to force PCI devices to be page aligned").

Feedback is appreciated.

Thanks,
Shawn

Shawn Anastasio (3):
  PCI: Introduce pcibios_ignore_alignment_request
  powerpc/64: Enable pcibios_after_init hook on ppc64
  powerpc/pseries: Allow user-specified PCI resource alignment after
    init

 arch/powerpc/include/asm/machdep.h     |  6 ++++--
 arch/powerpc/kernel/pci-common.c       |  9 +++++++++
 arch/powerpc/kernel/pci_64.c           |  4 ++++
 arch/powerpc/platforms/pseries/setup.c | 22 ++++++++++++++++++++++
 drivers/pci/pci.c                      |  9 +++++++--
 5 files changed, 46 insertions(+), 4 deletions(-)

-- 
2.20.1

[RESEND PATCH 1/3] PCI: Introduce pcibios_ignore_alignment_request

[Shawn Anastasio]

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 +++++++--
 1 file changed, 7 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;
 	}
 
-- 
2.20.1

[RESEND PATCH 2/3] powerpc/64: Enable pcibios_after_init hook on ppc64

[Shawn Anastasio]

Enable the pcibios_after_init hook on all powerpc platforms.
This hook is executed at the end of pcibios_init and was previously
only available on CONFIG_PPC32.

Since it is useful and not inherently limited to 32-bit mode,
remove the limitation and allow it on all powerpc platforms.

Signed-off-by: Shawn Anastasio <shawn@anastas.io>
---
 arch/powerpc/include/asm/machdep.h | 3 +--
 arch/powerpc/kernel/pci_64.c       | 4 ++++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index 2f0ca6560e47..2fbfaa9176ed 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -150,6 +150,7 @@ struct machdep_calls {
 	void		(*init)(void);
 
 	void		(*kgdb_map_scc)(void);
+#endif /* CONFIG_PPC32 */
 
 	/*
 	 * optional PCI "hooks"
@@ -157,8 +158,6 @@ struct machdep_calls {
 	/* Called at then very end of pcibios_init() */
 	void (*pcibios_after_init)(void);
 
-#endif /* CONFIG_PPC32 */
-
 	/* Called in indirect_* to avoid touching devices */
 	int (*pci_exclude_device)(struct pci_controller *, unsigned char, unsigned char);
 
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 9d8c10d55407..fba7fe6e4a50 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -68,6 +68,10 @@ static int __init pcibios_init(void)
 
 	printk(KERN_DEBUG "PCI: Probing PCI hardware done\n");
 
+	/* Call machine dependent post-init code */
+	if (ppc_md.pcibios_after_init)
+		ppc_md.pcibios_after_init();
+
 	return 0;
 }
 
-- 
2.20.1

[RESEND PATCH 3/3] powerpc/pseries: Allow user-specified PCI resource alignment after init

[Shawn Anastasio]

On pseries, custom PCI resource alignment specified with the commandline
argument pci=resource_alignment is disabled due to PCI resources being
managed by the firmware. However, in the case of PCI hotplug the
resources are managed by the kernel, so custom alignments should be
honored in these cases. This is done by only honoring custom
alignments after initial PCI initialization is done, to ensure that
all devices managed by the firmware are excluded.

Without this ability, sub-page BARs sometimes get mapped in between
page boundaries for hotplugged devices and are therefore unusable
with the VFIO framework. This change allows users to request
page alignment for devices they wish to access via VFIO using
the pci=resource_alignment commandline argument.

In the future, this could be extended to provide page-aligned
resources by default for hotplugged devices, similar to what is
done on powernv by commit 382746376993 ("powerpc/powernv: Override
pcibios_default_alignment() to force PCI devices to be page aligned")

Signed-off-by: Shawn Anastasio <shawn@anastas.io>
---
 arch/powerpc/include/asm/machdep.h     |  3 +++
 arch/powerpc/kernel/pci-common.c       |  9 +++++++++
 arch/powerpc/platforms/pseries/setup.c | 22 ++++++++++++++++++++++
 3 files changed, 34 insertions(+)

diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index 2fbfaa9176ed..46eb62c0954e 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -179,6 +179,9 @@ struct machdep_calls {
 
 	resource_size_t (*pcibios_default_alignment)(void);
 
+	/* Called when determining PCI resource alignment */
+	int (*pcibios_ignore_alignment_request)(void);
+
 #ifdef CONFIG_PCI_IOV
 	void (*pcibios_fixup_sriov)(struct pci_dev *pdev);
 	resource_size_t (*pcibios_iov_resource_alignment)(struct pci_dev *, int resno);
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index ff4b7539cbdf..1a6ded45a701 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -238,6 +238,15 @@ resource_size_t pcibios_default_alignment(void)
 	return 0;
 }
 
+resource_size_t pcibios_ignore_alignment_request(void)
+{
+	if (ppc_md.pcibios_ignore_alignment_request)
+		return ppc_md.pcibios_ignore_alignment_request();
+
+	/* Fall back to default method of checking PCI_PROBE_ONLY */
+	return pci_has_flag(PCI_PROBE_ONLY);
+}
+
 #ifdef CONFIG_PCI_IOV
 resource_size_t pcibios_iov_resource_alignment(struct pci_dev *pdev, int resno)
 {
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index e4f0dfd4ae33..c6af2ed8ee0f 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -82,6 +82,8 @@ EXPORT_SYMBOL(CMO_PageSize);
 
 int fwnmi_active;  /* TRUE if an FWNMI handler is present */
 
+int initial_pci_init_done; /* TRUE if initial pcibios init has completed */
+
 static void pSeries_show_cpuinfo(struct seq_file *m)
 {
 	struct device_node *root;
@@ -749,6 +751,23 @@ static resource_size_t pseries_pci_iov_resource_alignment(struct pci_dev *pdev,
 }
 #endif
 
+static void pseries_after_init(void)
+{
+	initial_pci_init_done = 1;
+}
+
+static int pseries_ignore_alignment_request(void)
+{
+	if (initial_pci_init_done)
+		/*
+		 * Allow custom alignments after init for things
+		 * like PCI hotplugging.
+		 */
+		return 0;
+
+	return pci_has_flag(PCI_PROBE_ONLY);
+}
+
 static void __init pSeries_setup_arch(void)
 {
 	set_arch_panic_timeout(10, ARCH_PANIC_TIMEOUT);
@@ -797,6 +816,9 @@ static void __init pSeries_setup_arch(void)
 	}
 
 	ppc_md.pcibios_root_bridge_prepare = pseries_root_bridge_prepare;
+	ppc_md.pcibios_after_init = pseries_after_init;
+	ppc_md.pcibios_ignore_alignment_request =
+		pseries_ignore_alignment_request;
 }
 
 static void pseries_panic(char *str)
-- 
2.20.1

Re: [RESEND PATCH 0/3] Allow custom PCI resource alignment on pseries

[Oliver]

On Tue, May 28, 2019 at 8:56 AM Shawn Anastasio <shawn@anastas.io> wrote:
>
> Hello all,
>
> This patch set implements support for user-specified PCI resource
> alignment on the pseries platform for hotplugged PCI devices.
> Currently on pseries, PCI resource alignments specified with the
> pci=resource_alignment commandline argument are ignored, since
> the firmware is in charge of managing the PCI resources. In the
> case of hotplugged devices, though, the kernel is in charge of
> configuring the resources and should obey alignment requirements.

Are you using hotplug to work around SLOF (the OF we use under qemu)
not aligning BARs to 64K? It looks like there is a commit in SLOF to
fix that (https://git.qemu.org/?p=SLOF.git;a=commit;f=board-qemu/slof/pci-phb.fs;h=1903174472f8800caf50c959b304501b4c01153c).

> The current behavior of ignoring the alignment for hotplugged devices
> results in sub-page BARs landing between page boundaries and
> becoming un-mappable from userspace via the VFIO framework.
> This issue was observed on a pseries KVM guest with hotplugged
> ivshmem devices.

> With these changes, users can specify an appropriate
> pci=resource_alignment argument on boot for devices they wish to use
> with VFIO.
>
> In the future, this could be extended to provide page-aligned
> resources by default for hotplugged devices, similar to what is done
> on powernv by commit 382746376993 ("powerpc/powernv: Override
> pcibios_default_alignment() to force PCI devices to be page aligned").

Can we make aligning the BARs to PAGE_SIZE the default behaviour? The
BAR assignment process is complex enough as-is so I'd rather we didn't
add another platform hack into the mix.

> Feedback is appreciated.
>
> Thanks,
> Shawn
>
> Shawn Anastasio (3):
>   PCI: Introduce pcibios_ignore_alignment_request
>   powerpc/64: Enable pcibios_after_init hook on ppc64
>   powerpc/pseries: Allow user-specified PCI resource alignment after
>     init
>
>  arch/powerpc/include/asm/machdep.h     |  6 ++++--
>  arch/powerpc/kernel/pci-common.c       |  9 +++++++++
>  arch/powerpc/kernel/pci_64.c           |  4 ++++
>  arch/powerpc/platforms/pseries/setup.c | 22 ++++++++++++++++++++++
>  drivers/pci/pci.c                      |  9 +++++++--
>  5 files changed, 46 insertions(+), 4 deletions(-)
>
> --
> 2.20.1
>

Re: [RESEND PATCH 0/3] Allow custom PCI resource alignment on pseries

[Shawn Anastasio]

On 5/27/19 11:01 PM, Oliver wrote:
> On Tue, May 28, 2019 at 8:56 AM Shawn Anastasio <shawn@anastas.io> wrote:
>>
>> Hello all,
>>
>> This patch set implements support for user-specified PCI resource
>> alignment on the pseries platform for hotplugged PCI devices.
>> Currently on pseries, PCI resource alignments specified with the
>> pci=resource_alignment commandline argument are ignored, since
>> the firmware is in charge of managing the PCI resources. In the
>> case of hotplugged devices, though, the kernel is in charge of
>> configuring the resources and should obey alignment requirements.
> 
> Are you using hotplug to work around SLOF (the OF we use under qemu)
> not aligning BARs to 64K? It looks like there is a commit in SLOF to
> fix that (https://git.qemu.org/?p=SLOF.git;a=commit;f=board-qemu/slof/pci-phb.fs;h=1903174472f8800caf50c959b304501b4c01153c).
> 

No, my application actually requires PCI hotplug at run-time.

>> The current behavior of ignoring the alignment for hotplugged devices
>> results in sub-page BARs landing between page boundaries and
>> becoming un-mappable from userspace via the VFIO framework.
>> This issue was observed on a pseries KVM guest with hotplugged
>> ivshmem devices.
> 
>> With these changes, users can specify an appropriate
>> pci=resource_alignment argument on boot for devices they wish to use
>> with VFIO.
>>
>> In the future, this could be extended to provide page-aligned
>> resources by default for hotplugged devices, similar to what is done
>> on powernv by commit 382746376993 ("powerpc/powernv: Override
>> pcibios_default_alignment() to force PCI devices to be page aligned").
> 
> Can we make aligning the BARs to PAGE_SIZE the default behaviour? The
> BAR assignment process is complex enough as-is so I'd rather we didn't
> add another platform hack into the mix.

Absolutely. This will still require the existing changes so that the 
custom alignment isn't flat-out ignored on pseries, but I can set
it to default to PAGE_SIZE as well, similar to how it's done on PowerNV.
I've just pushed a v3 to fix a typo and I'll incorporate this change
in v4.

>> Feedback is appreciated.
>>
>> Thanks,
>> Shawn
>>
>> Shawn Anastasio (3):
>>    PCI: Introduce pcibios_ignore_alignment_request
>>    powerpc/64: Enable pcibios_after_init hook on ppc64
>>    powerpc/pseries: Allow user-specified PCI resource alignment after
>>      init
>>
>>   arch/powerpc/include/asm/machdep.h     |  6 ++++--
>>   arch/powerpc/kernel/pci-common.c       |  9 +++++++++
>>   arch/powerpc/kernel/pci_64.c           |  4 ++++
>>   arch/powerpc/platforms/pseries/setup.c | 22 ++++++++++++++++++++++
>>   drivers/pci/pci.c                      |  9 +++++++--
>>   5 files changed, 46 insertions(+), 4 deletions(-)
>>
>> --
>> 2.20.1
>>

Re: [RESEND PATCH 0/3] Allow custom PCI resource alignment on pseries

[Oliver]

On Tue, May 28, 2019 at 2:09 PM Shawn Anastasio <shawn@anastas.io> wrote:
>
>
>
> On 5/27/19 11:01 PM, Oliver wrote:
> > On Tue, May 28, 2019 at 8:56 AM Shawn Anastasio <shawn@anastas.io> wrote:
> >>
> >> Hello all,
> >>
> >> This patch set implements support for user-specified PCI resource
> >> alignment on the pseries platform for hotplugged PCI devices.
> >> Currently on pseries, PCI resource alignments specified with the
> >> pci=resource_alignment commandline argument are ignored, since
> >> the firmware is in charge of managing the PCI resources. In the
> >> case of hotplugged devices, though, the kernel is in charge of
> >> configuring the resources and should obey alignment requirements.
> >
> > Are you using hotplug to work around SLOF (the OF we use under qemu)
> > not aligning BARs to 64K? It looks like there is a commit in SLOF to
> > fix that (https://git.qemu.org/?p=SLOF.git;a=commit;f=board-qemu/slof/pci-phb.fs;h=1903174472f8800caf50c959b304501b4c01153c).
> >
>
> No, my application actually requires PCI hotplug at run-time.
>
> >> The current behavior of ignoring the alignment for hotplugged devices
> >> results in sub-page BARs landing between page boundaries and
> >> becoming un-mappable from userspace via the VFIO framework.
> >> This issue was observed on a pseries KVM guest with hotplugged
> >> ivshmem devices.
> >
> >> With these changes, users can specify an appropriate
> >> pci=resource_alignment argument on boot for devices they wish to use
> >> with VFIO.
> >>
> >> In the future, this could be extended to provide page-aligned
> >> resources by default for hotplugged devices, similar to what is done
> >> on powernv by commit 382746376993 ("powerpc/powernv: Override
> >> pcibios_default_alignment() to force PCI devices to be page aligned").
> >
> > Can we make aligning the BARs to PAGE_SIZE the default behaviour? The
> > BAR assignment process is complex enough as-is so I'd rather we didn't
> > add another platform hack into the mix.
>
> Absolutely. This will still require the existing changes so that the
> custom alignment isn't flat-out ignored on pseries, but I can set
> it to default to PAGE_SIZE as well, similar to how it's done on PowerNV.
> I've just pushed a v3 to fix a typo and I'll incorporate this change
> in v4.

I was thinking we could get rid of the ppcmd callback and do it in
kernel/pci-common.c. PowerNV is the only platform that implements the
callback and the pseries implementation is going to be identical so I
don't think there's much of point in keeping the callback.

> >> Feedback is appreciated.
> >>
> >> Thanks,
> >> Shawn
> >>
> >> Shawn Anastasio (3):
> >>    PCI: Introduce pcibios_ignore_alignment_request
> >>    powerpc/64: Enable pcibios_after_init hook on ppc64
> >>    powerpc/pseries: Allow user-specified PCI resource alignment after
> >>      init
> >>
> >>   arch/powerpc/include/asm/machdep.h     |  6 ++++--
> >>   arch/powerpc/kernel/pci-common.c       |  9 +++++++++
> >>   arch/powerpc/kernel/pci_64.c           |  4 ++++
> >>   arch/powerpc/platforms/pseries/setup.c | 22 ++++++++++++++++++++++
> >>   drivers/pci/pci.c                      |  9 +++++++--
> >>   5 files changed, 46 insertions(+), 4 deletions(-)
> >>
> >> --
> >> 2.20.1
> >>