Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1622311

Re: [PATCH 2/4] PCI: add functionality for resizing resources v2

From Bjorn Helgaas <helgaas@kernel.org>
Newsgroups linux.kernel
Subject Re: [PATCH 2/4] PCI: add functionality for resizing resources v2
Date 2017-04-12 18:40 +0200
Message-ID <tvttf-6Mv-1@gated-at.bofh.it> (permalink)
References <tkxAe-ys-17@gated-at.bofh.it> <tkxAe-ys-23@gated-at.bofh.it> <toF6a-7By-29@gated-at.bofh.it> <tv63D-nH-13@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Tue, Apr 11, 2017 at 05:37:04PM +0200, Christian König wrote:

> >>+int pci_resize_resource(struct pci_dev *dev, int resno, int size)
> >>+{
> >>+	struct resource *res = dev->resource + resno;
> >>+	u32 sizes = pci_rbar_get_possible_sizes(dev, resno);
> >>+	int old = pci_rbar_get_current_size(dev, resno);
> >>+	u64 bytes = 1ULL << (size + 20);
> >>+	int ret = 0;
> >I think we should fail the request if the device is enabled, i.e., if
> >the PCI_COMMAND_MEMORY bit is set.  We can't safely change the BAR
> >while memory decoding is enabled.
> >
> >I know there's code in pci_std_update_resource() that turns off
> >PCI_COMMAND_MEMORY, but I think that's a mistake: I think it should
> >fail when asked to update an enabled BAR the same way
> >pci_iov_update_resource() does.
> >
> >I'm not sure why you call pci_reenable_device() below, but I think I
> >would rather have the driver do something like this:
> >
> >   pci_disable_device(dev);
> >   pci_resize_resource(dev, 0, size);
> >   pci_enable_device(dev);
> >
> >That way it's very clear to the driver that it must re-read all BARs
> >after resizing this one.
> 
> I've tried it, but this actually doesn't seem to work.
> 
> At least on the board I've tried pci_disable_device() doesn't clear
> the PCI_COMMAND_MEMORY bit, it just clears the master bit.

Yeah, you're right.  We generally turn *on* PCI_COMMAND_MEMORY in the
pci_enable_device() path, but the pci_disable_device() path doesn't
turn it off.

> Additional to that the power management reference counting
> pci_disable_device() and pci_enable_device() doesn't look like what
> I want for this functionality.
> 
> How about the driver needs to disabled memory decoding itself before
> trying to resize anything?

There are a few places that do that, so maybe that's the best option.

Bjorn

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

Re: [PATCH 2/4] PCI: add functionality for resizing resources v2 Christian König <deathsimple@vodafone.de> - 2017-04-11 17:40 +0200
  Re: [PATCH 2/4] PCI: add functionality for resizing resources v2 Bjorn Helgaas <helgaas@kernel.org> - 2017-04-12 18:40 +0200

csiph-web