On Tue, 2021-10-26 at 16:36 +0200, Lukasz Maniak wrote:
> On Thu, Oct 07, 2021 at 06:12:41PM -0400, Michael S. Tsirkin wrote:
> > On Thu, Oct 07, 2021 at 06:23:52PM +0200, Lukasz Maniak wrote:
> > > From: Knut Omang <knut.omang@oracle.com>
> > >
> > > Make the default PCI Express Capability for PCIe devices set
> > > MaxReadReq to 512.
> >
> > code says 256
> >
> > > Tyipcal modern devices people would want to
> >
> >
> > typo
> >
> > > emulate or simulate would want this. The previous value would
> > > cause warnings from the root port driver on some kernels.
> >
> >
> > which specifically?
> >
> > >
> > > Signed-off-by: Knut Omang <knuto@ifi.uio.no>
> >
> > we can't make changes like this unconditionally, this will
> > break migration across versions.
> > Pls tie this to a machine version.
> >
> > Thanks!
> > > ---
> > > hw/pci/pcie.c | 5 ++++-
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > > index 6e95d82903..c1a12f3744 100644
> > > --- a/hw/pci/pcie.c
> > > +++ b/hw/pci/pcie.c
> > > @@ -62,8 +62,9 @@ pcie_cap_v1_fill(PCIDevice *dev, uint8_t port, uint8_t type, uint8_t
> > > version)
> > > * Functions conforming to the ECN, PCI Express Base
> > > * Specification, Revision 1.1., or subsequent PCI Express Base
> > > * Specification revisions.
> > > + * + set max payload size to 256, which seems to be a common value
> > > */
> > > - pci_set_long(exp_cap + PCI_EXP_DEVCAP, PCI_EXP_DEVCAP_RBER);
> > > + pci_set_long(exp_cap + PCI_EXP_DEVCAP, PCI_EXP_DEVCAP_RBER | (0x1 &
> > > PCI_EXP_DEVCAP_PAYLOAD));
> > >
> > > pci_set_long(exp_cap + PCI_EXP_LNKCAP,
> > > (port << PCI_EXP_LNKCAP_PN_SHIFT) |
> > > @@ -179,6 +180,8 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset,
> > > pci_set_long(exp_cap + PCI_EXP_DEVCAP2,
> > > PCI_EXP_DEVCAP2_EFF | PCI_EXP_DEVCAP2_EETLPP);
> > >
> > > + pci_set_word(exp_cap + PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_READRQ_256B);
> > > +
> > > pci_set_word(dev->wmask + pos + PCI_EXP_DEVCTL2, PCI_EXP_DEVCTL2_EETLPPB);
> > >
> > > if (dev->cap_present & QEMU_PCIE_EXTCAP_INIT) {
> > > --
> > > 2.25.1
> >
>
> Hi Michael,
>
> The reason Knut keeps rebasing this fix along with SR-IOV patch is not
> clear for us.
Sorry for the slow response - I seem to have messed up my mail filters so this
thread slipped past my attention.
> Since we have tested the NVMe device without this fix and did not notice
> any issues mentioned by Knut on kernel 5.4.0, we decided to drop it for
> v2.
I agree, let's just drop it.
It was likely in the 3.x kernels I had to relate to back then,
likely discovered in Oracle Linux given that I did not specifically point to a kernel
range already back then.
> However, I have posted your comments to this patch on Knut's github so
> they can be addressed in case Knut decides to resubmit it later though.
Thanks for that ping, Lukasz, and great to see the patch finally being used in a
functional device!
Knut
> Thanks,
> Lukasz