Re: [PATCH 01/15] pcie: Set default and supported MaxReadReq to 512

[Lukasz Maniak]

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.

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.

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,
Lukasz