[RESEND] [SRU Artful/Zesty][PATCH] On ARM64 PCIE physical function passthrough guest fails to boot

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

[RESEND] [SRU Artful/Zesty][PATCH] On ARM64 PCIE physical function passthrough guest fails to boot

Manoj Iyer
Resending SRU request.

Please consider these patches that fixs bug https://launchpad.net/bugs/1732804.  The patches were cleanly cherry-picked from linux-next and a test kernel is available in the PPA https://launchpad.net/~centriq-team/+archive/ubuntu/lp1732804/ 
The kernel was tested by me as well as engg at Qualcomm on QDF2400 platform, and the results are posted to the bug report.

Thanks
Manoj Iyer


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] vfio/pci: Virtualize Maximum Payload Size

Manoj Iyer
From: Alex Williamson <[hidden email]>

With virtual PCI-Express chipsets, we now see userspace/guest drivers
trying to match the physical MPS setting to a virtual downstream port.
Of course a lone physical device surrounded by virtual interconnects
cannot make a correct decision for a proper MPS setting.  Instead,
let's virtualize the MPS control register so that writes through to
hardware are disallowed.  Userspace drivers like QEMU assume they can
write anything to the device and we'll filter out anything dangerous.
Since mismatched MPS can lead to AER and other faults, let's add it
to the kernel side rather than relying on userspace virtualization to
handle it.

BugLink: https://launchpad.net/bugs/1732804

Signed-off-by: Alex Williamson <[hidden email]>
Reviewed-by: Eric Auger <[hidden email]>
(cherry picked from linux-next commit 523184972b282cd9ca17a76f6ca4742394856818)
Signed-off-by: Manoj Iyer <[hidden email]>
---
 drivers/vfio/pci/vfio_pci_config.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
index 5628fe114347..91335e6de88a 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -849,11 +849,13 @@ static int __init init_pci_cap_exp_perm(struct perm_bits *perm)
 
  /*
  * Allow writes to device control fields, except devctl_phantom,
- * which could confuse IOMMU, and the ARI bit in devctl2, which
+ * which could confuse IOMMU, MPS, which can break communication
+ * with other physical devices, and the ARI bit in devctl2, which
  * is set at probe time.  FLR gets virtualized via our writefn.
  */
  p_setw(perm, PCI_EXP_DEVCTL,
-       PCI_EXP_DEVCTL_BCR_FLR, ~PCI_EXP_DEVCTL_PHANTOM);
+       PCI_EXP_DEVCTL_BCR_FLR | PCI_EXP_DEVCTL_PAYLOAD,
+       ~PCI_EXP_DEVCTL_PHANTOM);
  p_setw(perm, PCI_EXP_DEVCTL2, NO_VIRT, ~PCI_EXP_DEVCTL2_ARI);
  return 0;
 }
--
2.14.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] vfio/pci: Virtualize Maximum Read Request Size

Manoj Iyer
In reply to this post by Manoj Iyer
From: Alex Williamson <[hidden email]>

MRRS defines the maximum read request size a device is allowed to
make.  Drivers will often increase this to allow more data transfer
with a single request.  Completions to this request are bound by the
MPS setting for the bus.  Aside from device quirks (none known), it
doesn't seem to make sense to set an MRRS value less than MPS, yet
this is a likely scenario given that user drivers do not have a
system-wide view of the PCI topology.  Virtualize MRRS such that the
user can set MRRS >= MPS, but use MPS as the floor value that we'll
write to hardware.

BugLink: https://launchpad.net/bugs/1732804

Signed-off-by: Alex Williamson <[hidden email]>
(cherry picked from linux-next commit cf0d53ba4947aad6e471491d5b20a567cbe92e56)
Signed-off-by: Manoj Iyer <[hidden email]>
---
 drivers/vfio/pci/vfio_pci_config.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
index 91335e6de88a..115a36f6f403 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -808,6 +808,7 @@ static int vfio_exp_config_write(struct vfio_pci_device *vdev, int pos,
 {
  __le16 *ctrl = (__le16 *)(vdev->vconfig + pos -
   offset + PCI_EXP_DEVCTL);
+ int readrq = le16_to_cpu(*ctrl) & PCI_EXP_DEVCTL_READRQ;
 
  count = vfio_default_config_write(vdev, pos, count, perm, offset, val);
  if (count < 0)
@@ -833,6 +834,27 @@ static int vfio_exp_config_write(struct vfio_pci_device *vdev, int pos,
  pci_try_reset_function(vdev->pdev);
  }
 
+ /*
+ * MPS is virtualized to the user, writes do not change the physical
+ * register since determining a proper MPS value requires a system wide
+ * device view.  The MRRS is largely independent of MPS, but since the
+ * user does not have that system-wide view, they might set a safe, but
+ * inefficiently low value.  Here we allow writes through to hardware,
+ * but we set the floor to the physical device MPS setting, so that
+ * we can at least use full TLPs, as defined by the MPS value.
+ *
+ * NB, if any devices actually depend on an artificially low MRRS
+ * setting, this will need to be revisited, perhaps with a quirk
+ * though pcie_set_readrq().
+ */
+ if (readrq != (le16_to_cpu(*ctrl) & PCI_EXP_DEVCTL_READRQ)) {
+ readrq = 128 <<
+ ((le16_to_cpu(*ctrl) & PCI_EXP_DEVCTL_READRQ) >> 12);
+ readrq = max(readrq, pcie_get_mps(vdev->pdev));
+
+ pcie_set_readrq(vdev->pdev, readrq);
+ }
+
  return count;
 }
 
@@ -851,11 +873,12 @@ static int __init init_pci_cap_exp_perm(struct perm_bits *perm)
  * Allow writes to device control fields, except devctl_phantom,
  * which could confuse IOMMU, MPS, which can break communication
  * with other physical devices, and the ARI bit in devctl2, which
- * is set at probe time.  FLR gets virtualized via our writefn.
+ * is set at probe time.  FLR and MRRS get virtualized via our
+ * writefn.
  */
  p_setw(perm, PCI_EXP_DEVCTL,
-       PCI_EXP_DEVCTL_BCR_FLR | PCI_EXP_DEVCTL_PAYLOAD,
-       ~PCI_EXP_DEVCTL_PHANTOM);
+       PCI_EXP_DEVCTL_BCR_FLR | PCI_EXP_DEVCTL_PAYLOAD |
+       PCI_EXP_DEVCTL_READRQ, ~PCI_EXP_DEVCTL_PHANTOM);
  p_setw(perm, PCI_EXP_DEVCTL2, NO_VIRT, ~PCI_EXP_DEVCTL2_ARI);
  return 0;
 }
--
2.14.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [RESEND] [SRU Artful/Zesty][PATCH] On ARM64 PCIE physical function passthrough guest fails to boot

Seth Forshee
In reply to this post by Manoj Iyer
On Wed, Jan 03, 2018 at 01:37:31PM -0600, Manoj Iyer wrote:
> Resending SRU request.
>
> Please consider these patches that fixs bug https://launchpad.net/bugs/1732804.  The patches were cleanly cherry-picked from linux-next and a test kernel is available in the PPA https://launchpad.net/~centriq-team/+archive/ubuntu/lp1732804/ 
> The kernel was tested by me as well as engg at Qualcomm on QDF2400 platform, and the results are posted to the bug report.

Any particular reason you didn't include bionic here? Normally we'd
expect things going into the latest stable kernel to also go into the
devel kernel. Bionic already has the first patch from 4.14 stable
updates, but not the second.

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [RESEND] [SRU Artful/Zesty][PATCH] On ARM64 PCIE physical function passthrough guest fails to boot

Seth Forshee
On Fri, Jan 05, 2018 at 09:47:48AM -0600, Seth Forshee wrote:

> On Wed, Jan 03, 2018 at 01:37:31PM -0600, Manoj Iyer wrote:
> > Resending SRU request.
> >
> > Please consider these patches that fixs bug https://launchpad.net/bugs/1732804.  The patches were cleanly cherry-picked from linux-next and a test kernel is available in the PPA https://launchpad.net/~centriq-team/+archive/ubuntu/lp1732804/ 
> > The kernel was tested by me as well as engg at Qualcomm on QDF2400 platform, and the results are posted to the bug report.
>
> Any particular reason you didn't include bionic here? Normally we'd
> expect things going into the latest stable kernel to also go into the
> devel kernel. Bionic already has the first patch from 4.14 stable
> updates, but not the second.

Also noting that while the patches say cherry picked from linux-next
they are actually in Linus' tree as of 4.15-rc1.

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [RESEND] [SRU Artful/Zesty][PATCH] On ARM64 PCIE physical function passthrough guest fails to boot

Manoj Iyer
In reply to this post by Seth Forshee

At the time I submitted the patch (Nov) I think the bionic tree was just
Artful.

On Fri, 5 Jan 2018, Seth Forshee wrote:

> On Wed, Jan 03, 2018 at 01:37:31PM -0600, Manoj Iyer wrote:
>> Resending SRU request.
>>
>> Please consider these patches that fixs bug https://launchpad.net/bugs/1732804.  The patches were cleanly cherry-picked from linux-next and a test kernel is available in the PPA https://launchpad.net/~centriq-team/+archive/ubuntu/lp1732804/
>> The kernel was tested by me as well as engg at Qualcomm on QDF2400 platform, and the results are posted to the bug report.
>
> Any particular reason you didn't include bionic here? Normally we'd
> expect things going into the latest stable kernel to also go into the
> devel kernel. Bionic already has the first patch from 4.14 stable
> updates, but not the second.
>
>

--
============================
Manoj Iyer
Ubuntu/Canonical
ARM Servers - Cloud
============================

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [RESEND] [SRU Artful/Zesty][PATCH] On ARM64 PCIE physical function passthrough guest fails to boot

Manoj Iyer
In reply to this post by Seth Forshee
On Fri, 5 Jan 2018, Seth Forshee wrote:

> On Fri, Jan 05, 2018 at 09:47:48AM -0600, Seth Forshee wrote:
>> On Wed, Jan 03, 2018 at 01:37:31PM -0600, Manoj Iyer wrote:
>>> Resending SRU request.
>>>
>>> Please consider these patches that fixs bug https://launchpad.net/bugs/1732804.  The patches were cleanly cherry-picked from linux-next and a test kernel is available in the PPA https://launchpad.net/~centriq-team/+archive/ubuntu/lp1732804/
>>> The kernel was tested by me as well as engg at Qualcomm on QDF2400 platform, and the results are posted to the bug report.
>>
>> Any particular reason you didn't include bionic here? Normally we'd
>> expect things going into the latest stable kernel to also go into the
>> devel kernel. Bionic already has the first patch from 4.14 stable
>> updates, but not the second.
>
> Also noting that while the patches say cherry picked from linux-next
> they are actually in Linus' tree as of 4.15-rc1.

At the time I submitted these patches (Nov) these were in linux-next. I
resent those same patches again after talking to jsalisbury because these
patches were missed in the last SRU cycle. Would you want me to resubmit
with cherry picks from Linus tree? let me know and I can do that.

>
>

--
============================
Manoj Iyer
Ubuntu/Canonical
ARM Servers - Cloud
============================

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED[B]: [RESEND] [SRU Artful/Zesty][PATCH] On ARM64 PCIE physical function passthrough guest fails to boot

Seth Forshee
In reply to this post by Manoj Iyer
On Fri, Jan 05, 2018 at 10:09:24AM -0600, Manoj Iyer wrote:
>
> At the time I submitted the patch (Nov) I think the bionic tree was just
> Artful.

But you resent just a couple of days ago ;-)

Anyway, I've cherry picked the second patch to bionic/master-next.  In
the future please try to remember to evaluate whether patches are also
needed in the devel or unstable trees when sending for SRU. Thanks!

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [RESEND] [SRU Artful/Zesty][PATCH] On ARM64 PCIE physical function passthrough guest fails to boot

Seth Forshee
In reply to this post by Manoj Iyer
On Fri, Jan 05, 2018 at 10:14:09AM -0600, Manoj Iyer wrote:

> On Fri, 5 Jan 2018, Seth Forshee wrote:
>
> > On Fri, Jan 05, 2018 at 09:47:48AM -0600, Seth Forshee wrote:
> > > On Wed, Jan 03, 2018 at 01:37:31PM -0600, Manoj Iyer wrote:
> > > > Resending SRU request.
> > > >
> > > > Please consider these patches that fixs bug https://launchpad.net/bugs/1732804.  The patches were cleanly cherry-picked from linux-next and a test kernel is available in the PPA https://launchpad.net/~centriq-team/+archive/ubuntu/lp1732804/
> > > > The kernel was tested by me as well as engg at Qualcomm on QDF2400 platform, and the results are posted to the bug report.
> > >
> > > Any particular reason you didn't include bionic here? Normally we'd
> > > expect things going into the latest stable kernel to also go into the
> > > devel kernel. Bionic already has the first patch from 4.14 stable
> > > updates, but not the second.
> >
> > Also noting that while the patches say cherry picked from linux-next
> > they are actually in Linus' tree as of 4.15-rc1.
>
> At the time I submitted these patches (Nov) these were in linux-next. I
> resent those same patches again after talking to jsalisbury because these
> patches were missed in the last SRU cycle. Would you want me to resubmit
> with cherry picks from Linus tree? let me know and I can do that.

I assume whoever applies these to the stable tree will be kind enough to
fix that for you, but that's up to them.

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: APPLIED[B]: [RESEND] [SRU Artful/Zesty][PATCH] On ARM64 PCIE physical function passthrough guest fails to boot

Manoj Iyer
In reply to this post by Seth Forshee
On Fri, 5 Jan 2018, Seth Forshee wrote:

> On Fri, Jan 05, 2018 at 10:09:24AM -0600, Manoj Iyer wrote:
>>
>> At the time I submitted the patch (Nov) I think the bionic tree was just
>> Artful.
>
> But you resent just a couple of days ago ;-)
>
> Anyway, I've cherry picked the second patch to bionic/master-next.  In
> the future please try to remember to evaluate whether patches are also
> needed in the devel or unstable trees when sending for SRU. Thanks!

Thanks for applying that .. it was a resend of the original and honestly I
should have also added bionic to the list. Will pay attn in future.

>
>

--
============================
Manoj Iyer
Ubuntu/Canonical
ARM Servers - Cloud
============================

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [RESEND] [SRU Artful/Zesty][PATCH] On ARM64 PCIE physical function passthrough guest fails to boot

Paolo Pisati-5
In reply to this post by Manoj Iyer
same here - can you resend using the standard SRU format[1], using a
cover letter plus patches, cherry-picking from Linus tree instead of
linux-next?

1: https://wiki.ubuntu.com/Kernel/Dev/StablePatchFormat

On Fri, Jan 5, 2018 at 5:14 PM, Manoj Iyer <[hidden email]> wrote:

> On Fri, 5 Jan 2018, Seth Forshee wrote:
>
>> On Fri, Jan 05, 2018 at 09:47:48AM -0600, Seth Forshee wrote:
>>>
>>> On Wed, Jan 03, 2018 at 01:37:31PM -0600, Manoj Iyer wrote:
>>>>
>>>> Resending SRU request.
>>>>
>>>> Please consider these patches that fixs bug
>>>> https://launchpad.net/bugs/1732804.  The patches were cleanly cherry-picked
>>>> from linux-next and a test kernel is available in the PPA
>>>> https://launchpad.net/~centriq-team/+archive/ubuntu/lp1732804/
>>>> The kernel was tested by me as well as engg at Qualcomm on QDF2400
>>>> platform, and the results are posted to the bug report.
>>>
>>>
>>> Any particular reason you didn't include bionic here? Normally we'd
>>> expect things going into the latest stable kernel to also go into the
>>> devel kernel. Bionic already has the first patch from 4.14 stable
>>> updates, but not the second.
>>
>>
>> Also noting that while the patches say cherry picked from linux-next
>> they are actually in Linus' tree as of 4.15-rc1.
>
>
> At the time I submitted these patches (Nov) these were in linux-next. I
> resent those same patches again after talking to jsalisbury because these
> patches were missed in the last SRU cycle. Would you want me to resubmit
> with cherry picks from Linus tree? let me know and I can do that.
>
>>
>>
>
> --
> ============================
> Manoj Iyer
> Ubuntu/Canonical
> ARM Servers - Cloud
> ============================
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team



--
bye,
p.

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team