On 20/10, Rob Herring wrote:
> On Fri, Oct 15, 2021 at 08:08:44PM +0530, Naveen Naidu wrote:
> > An MMIO read from a PCI device that doesn't exist or doesn't respond
> > causes a PCI error. There's no real data to return to satisfy the
> > CPU read, so most hardware fabricates ~0 data.
> >
> > Use SET_PCI_ERROR_RESPONSE() to set the error response and
> > RESPONSE_IS_PCI_ERROR() to check the error response during hardware
> > read.
> >
> > These definitions make error checks consistent and easier to find.
> >
> > Signed-off-by: Naveen Naidu <naveennaidu479@xxxxxxxxx>
> > ---
> > drivers/pci/access.c | 18 +++++++++---------
> > 1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/pci/access.c b/drivers/pci/access.c
> > index b3b2006ed1d2..03712866c818 100644
> > --- a/drivers/pci/access.c
> > +++ b/drivers/pci/access.c
> > @@ -417,10 +417,10 @@ int pcie_capability_read_word(struct pci_dev *dev, int pos, u16 *val)
> > ret = pci_read_config_word(dev, pci_pcie_cap(dev) + pos, val);
> > /*
> > * Reset *val to 0 if pci_read_config_word() fails, it may
> > - * have been written as 0xFFFF if hardware error happens
> > - * during pci_read_config_word().
> > + * have been written as 0xFFFF (PCI_ERROR_RESPONSE) if hardware error
> > + * happens during pci_read_config_word().
> > */
> > - if (ret)
> > + if (RESPONSE_IS_PCI_ERROR(val))
>
> What if there is no error (in ret) and the register value was actually
> ~0? We'd be corrupting the value.
>
> In general, I think we should rely more on the error codes and less on
> the ~0 value.
>
Thank you for the review. I'll fix this up when I send v3 for this patch
series.
> > *val = 0;
> > return ret;
> > }
> > @@ -452,10 +452,10 @@ int pcie_capability_read_dword(struct pci_dev *dev, int pos, u32 *val)
> > ret = pci_read_config_dword(dev, pci_pcie_cap(dev) + pos, val);
> > /*
> > * Reset *val to 0 if pci_read_config_dword() fails, it may
> > - * have been written as 0xFFFFFFFF if hardware error happens
> > - * during pci_read_config_dword().
> > + * have been written as 0xFFFFFFFF (PCI_ERROR_RESPONSE) if hardware
> > + * error happens during pci_read_config_dword().
> > */
> > - if (ret)
> > + if (RESPONSE_IS_PCI_ERROR(val))
> > *val = 0;
> > return ret;
> > }
> > @@ -529,7 +529,7 @@ EXPORT_SYMBOL(pcie_capability_clear_and_set_dword);
> > int pci_read_config_byte(const struct pci_dev *dev, int where, u8 *val)
> > {
> > if (pci_dev_is_disconnected(dev)) {
> > - *val = ~0;
> > + SET_PCI_ERROR_RESPONSE(val);
> > return PCIBIOS_DEVICE_NOT_FOUND;
> > }
> > return pci_bus_read_config_byte(dev->bus, dev->devfn, where, val);
> > @@ -539,7 +539,7 @@ EXPORT_SYMBOL(pci_read_config_byte);
> > int pci_read_config_word(const struct pci_dev *dev, int where, u16 *val)
> > {
> > if (pci_dev_is_disconnected(dev)) {
> > - *val = ~0;
> > + SET_PCI_ERROR_RESPONSE(val);
> > return PCIBIOS_DEVICE_NOT_FOUND;
> > }
> > return pci_bus_read_config_word(dev->bus, dev->devfn, where, val);
> > @@ -550,7 +550,7 @@ int pci_read_config_dword(const struct pci_dev *dev, int where,
> > u32 *val)
> > {
> > if (pci_dev_is_disconnected(dev)) {
> > - *val = ~0;
> > + SET_PCI_ERROR_RESPONSE(val);
> > return PCIBIOS_DEVICE_NOT_FOUND;
> > }
> > return pci_bus_read_config_dword(dev->bus, dev->devfn, where, val);
> > --
> > 2.25.1
> >
> >
