PCI: Disable not requested resource types in pci_enable_resources
diff mbox series

Message ID 18bb3264-9901-135d-8b40-1ee98dd672f1@gmail.com
State New
Delegated to: Bjorn Helgaas
Headers show
Series
  • PCI: Disable not requested resource types in pci_enable_resources
Related show

Commit Message

Heiner Kallweit May 28, 2020, 6:47 p.m. UTC
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(-)

Comments

Bjorn Helgaas July 10, 2020, 9:09 p.m. UTC | #1
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
>
Heiner Kallweit July 15, 2020, 6:22 p.m. UTC | #2
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
>>

Patch
diff mbox series

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)))