PCI: Disable not requested resource types in pci_enable_resources

PCI: Disable not requested resource types in pci_enable_resources

[Heiner Kallweit]

Currently, if both resource types are enabled before the call, the mask
value doesn't matter. Means as of today I wouldn't be able to e.g.
disable PCI_COMMAND_IO. At least my interpretation is that mask defines
which resource types are enabled after the call. Therefore change the
behavior to disable not requested resource types.

At least on my x86 devices this change doesn't have side effects.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/pci/setup-res.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index d21fa04fa..6ef458c10 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -459,8 +459,8 @@ int pci_enable_resources(struct pci_dev *dev, int mask)
 	int i;
 	struct resource *r;
 
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	old_cmd = cmd;
+	pci_read_config_word(dev, PCI_COMMAND, &old_cmd);
+	cmd = old_cmd & ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
 
 	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
 		if (!(mask & (1 << i)))
-- 
2.26.2

Re: PCI: Disable not requested resource types in pci_enable_resources

[Bjorn Helgaas]

On Thu, May 28, 2020 at 08:47:12PM +0200, Heiner Kallweit wrote:
> Currently, if both resource types are enabled before the call, the mask
> value doesn't matter. Means as of today I wouldn't be able to e.g.
> disable PCI_COMMAND_IO. At least my interpretation is that mask defines
> which resource types are enabled after the call. Therefore change the
> behavior to disable not requested resource types.
> 
> At least on my x86 devices this change doesn't have side effects.

Does this have a practical benefit?  If it fixes a bug or if there's
something useful we can do because of this patch, I'll push it higher
up the priority list.

> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/pci/setup-res.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index d21fa04fa..6ef458c10 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -459,8 +459,8 @@ int pci_enable_resources(struct pci_dev *dev, int mask)
>  	int i;
>  	struct resource *r;
>  
> -	pci_read_config_word(dev, PCI_COMMAND, &cmd);
> -	old_cmd = cmd;
> +	pci_read_config_word(dev, PCI_COMMAND, &old_cmd);
> +	cmd = old_cmd & ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
>  
>  	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
>  		if (!(mask & (1 << i)))
> -- 
> 2.26.2
> 

Re: PCI: Disable not requested resource types in pci_enable_resources

[Heiner Kallweit]

On 10.07.2020 23:09, Bjorn Helgaas wrote:
> On Thu, May 28, 2020 at 08:47:12PM +0200, Heiner Kallweit wrote:
>> Currently, if both resource types are enabled before the call, the mask
>> value doesn't matter. Means as of today I wouldn't be able to e.g.
>> disable PCI_COMMAND_IO. At least my interpretation is that mask defines
>> which resource types are enabled after the call. Therefore change the
>> behavior to disable not requested resource types.
>>
>> At least on my x86 devices this change doesn't have side effects.
> 
> Does this have a practical benefit?  If it fixes a bug or if there's
> something useful we can do because of this patch, I'll push it higher
> up the priority list.
> 
There's no big benefit. The current behavior just doesn't seem to be
consistent, and I don't see why we have a function pci_enable_device_mem().
Also after calling this function IO resources can be active.
So why not remove this function and use pci_enable_device() always.

Small benefit is that the change allows to guarantee that IO resources
are disabled after calling pci_enable_device_mem().
This might help to avoid using IO resources mistakenly in a driver.

>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/pci/setup-res.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
>> index d21fa04fa..6ef458c10 100644
>> --- a/drivers/pci/setup-res.c
>> +++ b/drivers/pci/setup-res.c
>> @@ -459,8 +459,8 @@ int pci_enable_resources(struct pci_dev *dev, int mask)
>>  	int i;
>>  	struct resource *r;
>>  
>> -	pci_read_config_word(dev, PCI_COMMAND, &cmd);
>> -	old_cmd = cmd;
>> +	pci_read_config_word(dev, PCI_COMMAND, &old_cmd);
>> +	cmd = old_cmd & ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
>>  
>>  	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
>>  		if (!(mask & (1 << i)))
>> -- 
>> 2.26.2
>>