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

[Logan Gunthorpe <logang@deltatee.com>]

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:
>>> Hi Ben and Logan,
>>>
>>> It looks like my git send-email has been not working correctly since I
>>> started trying to get these patches accepted. I may have remedied this
>>> now, but I have seen that Logan tried to find these patches and failed.
>>> So as a courtesy until I post PATCH v7 (hopefully correctly, this time),
>>> I am forwarding you the patches. I hope you like them. I would love to 
>>> know of any concerns or questions you may have, and / or what happens if 
>>> you test them. Thanks and all the best!
>>>
>>> ----- 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.

Logan