| Message ID | 18bb3264-9901-135d-8b40-1ee98dd672f1@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: Disable not requested resource types in pci_enable_resources | expand |
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 >
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 >>
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)))
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(-)