Re: [nicholas.johnson-opensource@outlook.com.au: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently]

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Jun 19, 2019 at 07:35:21PM -0600, Logan Gunthorpe wrote:
> On 2019-06-19 6:56 p.m., Nicholas Johnson wrote:
> > On Wed, Jun 19, 2019 at 10:45:38AM -0600, Logan Gunthorpe wrote:
> >> On 2019-06-19 8:01 a.m., Nicholas Johnson wrote:
> >>> ----- Forwarded message from Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au> -----
> >>>
> >>> Date: Thu, 23 May 2019 06:29:28 +0800
> >>> From: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
> >>> To: linux-kernel@vger.kernel.org
> >>> Cc: linux-pci@vger.kernel.org, bhelgaas@google.com, mika.westerberg@linux.intel.com, corbet@lwn.net, Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
> >>> Subject: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently
> >>> X-Mailer: git-send-email 2.19.1
> >>>
> >>> Add kernel parameter pci=hpmemprefsize=nn[KMG] to control
> >>> MMIO_PREF size for PCI hotplug bridges.
> >>
> >> Makes sense.
> >>
> >>> Change behaviour of pci=hpmemsize=nn[KMG] to not set MMIO_PREF
> >>> size if hpmempref has been specified, rather than controlling
> >>> both MMIO and MMIO_PREF sizes unconditionally.
> >>
> >> I don't think I like that fact that hpmemsize behaves differently
> >> if hpmempref size is specfied before it. I'd probably suggest
> >> having three parameters: hpmemsize which sets both as it always
> >> has, a pref one and a regular one which each set one of
> >> parameters.
> > 
> > It does not matter if hpmempref is specified before or after
> > hpmemsize.  I made sure of that.
> 
> > Originally, I proposed to depreciate hpiosize, hpmemsize, and
> > introduce: hp_io_size, hp_mmio_size, hp_mmio_pref_size, each
> > controlling its own window exclusively.
> > 
> > The patch had the old parameters work with a warning, and if the
> > new ones were specified, they would override the old ones. Then,
> > after a few kernel releases, the old ones could be removed.
> 
> Well I don't like that either. No need to depreciate hpmemsize.
> 
> > Bjorn insisted that there be nil changes which break the existing
> > parameters, and the solution he requested was to leave hpmemsize
> > to work exactly the same (controlling both MMIO and MMIO_PREF),
> > unless hpmemprefsize is given, which will take control of
> > MMIO_PREF from hpmemsize.
> 
> I agree with Bjorn here too but my suggestion is to leave hpmemsize
> alone and have it set both values as it has always done. And add two
> new parameters to set one or the other. Then there's none of this
> "sets one if the other one wasn't set". Also, if I only want to
> change the non-preftechable version then your method leaves no way
> to do so without setting the preftechable version.

Adding two new parameters sounds like a good idea to me.