linux-kernel - Re: [PATCH] vfio/pci: Virtualize Maximum Payload Size

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170920101133.33b68d8f@w520.home>
Date:   Wed, 20 Sep 2017 10:11:33 -0600
From:   Alex Williamson <alex.williamson@...hat.com>
To:     Auger Eric <eric.auger@...hat.com>
Cc:     Sinan Kaya <okaya@...eaurora.org>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, jcm@...hat.com,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>
Subject: Re: [PATCH] vfio/pci: Virtualize Maximum Payload Size

On Wed, 20 Sep 2017 16:26:25 +0200
Auger Eric <eric.auger@...hat.com> wrote:

> Hi Sinan,
> 
> On 20/09/2017 15:01, Sinan Kaya wrote:
> > On 9/20/2017 3:59 AM, Auger Eric wrote:  
> >>> My impression is that MRRS is predominantly device and driver
> >>> dependent, not topology dependent.  A device can send a read request
> >>> with a size larger than MPS, which implies that the device supplying
> >>> the read data would split it into multiple TLPs based on MPS.  
> >> I read that too on the net. However in in 6.3.4.1. (3.0. Nov 10), Rules
> >> for SW Configuration it is written:
> >> "Software must set Max_Read_Request_Size of an isochronous-configured
> >> device with a value that does not exceed the Max_Payload_Size set for
> >> the device."
> >>
> >> But on the the other hand some drivers are setting the MMRS directly
> >> without further checking the MPS?  
> > 
> > We discussed this on LPC. MRRS and MPS are two independent concepts and
> > are not related to each other under normal circumstances. 
> > 
> > The only valid criteria is that MRRS needs to be a multiple of MPS.
> > 
> > https://linuxplumbersconf.org/2017/ocw//system/presentations/4732/original/crs.pdf
> > 
> > Because completions are required to be a minimum of MPS size. If MRRS > MPS,
> > read response is sent as multiple completions.  
> 
> With that patch, you can end up with MRRS < MPS. Do I understand
> correctly this is an issue?

My impression is that the issue would be inefficiency.  There should be
nothing functionally wrong with a read request less than MPS, but we're
not "filling" the TLP as much as the topology allows.  Is that your
understanding as well, Sinan?

It seems like it would be relatively easy to virtualize MRRS like we do
the FLR bit, ie. evaluate the change the user is trying to make and
update MRRS with pci-core callbacks, capping the lower bound equal to
MPS for efficiency.  It's possible we'll encounter devices that really
do need a lower MPS, but it seems unlikely since this is the setting
the PCI core seems to make by default (MRRS == MPS).  Thanks,

Alex