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@xxxxxxxxxxxxxx> ----- > >>> > >>> Date: Thu, 23 May 2019 06:29:28 +0800 > >>> From: Nicholas Johnson <nicholas.johnson-opensource@xxxxxxxxxxxxxx> > >>> To: linux-kernel@xxxxxxxxxxxxxxx > >>> Cc: linux-pci@xxxxxxxxxxxxxxx, bhelgaas@xxxxxxxxxx, mika.westerberg@xxxxxxxxxxxxxxx, corbet@xxxxxxx, Nicholas Johnson <nicholas.johnson-opensource@xxxxxxxxxxxxxx> > >>> 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.