| Message ID | 1473313320-27581-1-git-send-email-gwshan@linux.vnet.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
Hi, looks ok to me. Feel free to add your pick of: Reviewed-by: Alistair Popple <alistair@popple.id.au> Acked-by: Alistair Popple <alistair@popple.id.au> On Thu, 8 Sep 2016 03:42:00 PM Gavin Shan wrote: > The criteria to match the filter according to PCI config register > offset and length is strict one, meaning the accessed region should > matches with the filter's tracking region. However, the code isn't > correct enough - @len should be replaced with @pcrf->len. > > The strict matching has to account for the length of current PCI > config register access. We have to specify precise region through > @start and @len in pci_virt_add_filter() when creating the filter. > We lost flexibility. This changes the strict criteria to a relaxed > one - the matched filter is returned when the accessed region is > overlapped with the filter's region. With this change, the individual > filter's handlers should validate the accessed (register) region if > necessary. > > Fixes: 85daa7f00c50 ("core/pci: Support virtual device") > Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> > --- > core/pci-virt.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/core/pci-virt.c b/core/pci-virt.c > index 570c6e8..b531470 100644 > --- a/core/pci-virt.c > +++ b/core/pci-virt.c > @@ -55,9 +55,14 @@ static struct pci_cfg_reg_filter *pci_virt_find_filter( > if (!pvd || !len || start >= pvd->cfg_size) > return NULL; > > + /* Return filter if there is overlapped region. We don't > + * require strict matching for more flexibility. It also > + * means the associated handler should validate the register > + * offset and length. > + */ > list_for_each(&pvd->pcrf, pcrf, link) { > - if (start >= pcrf->start && > - (start + len) <= (pcrf->start + len)) > + if (start < (pcrf->start + pcrf->len) && > + (start + len) > pcrf->start) > return pcrf; > } > >
Gavin Shan <gwshan@linux.vnet.ibm.com> writes: > The criteria to match the filter according to PCI config register > offset and length is strict one, meaning the accessed region should > matches with the filter's tracking region. However, the code isn't > correct enough - @len should be replaced with @pcrf->len. > > The strict matching has to account for the length of current PCI > config register access. We have to specify precise region through > @start and @len in pci_virt_add_filter() when creating the filter. > We lost flexibility. This changes the strict criteria to a relaxed > one - the matched filter is returned when the accessed region is > overlapped with the filter's region. With this change, the individual > filter's handlers should validate the accessed (register) region if > necessary. > > Fixes: 85daa7f00c50 ("core/pci: Support virtual device") > Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> For some reason I marked this as "Accepted" in patchwork and didn't actually merge+push. My extreme bad. Anyway, merged to master as of e2fa37e75e428220940e0d901f0ad6d0cf0db37c
On Mon, Nov 07, 2016 at 05:56:58PM +1100, Stewart Smith wrote: >Gavin Shan <gwshan@linux.vnet.ibm.com> writes: >> The criteria to match the filter according to PCI config register >> offset and length is strict one, meaning the accessed region should >> matches with the filter's tracking region. However, the code isn't >> correct enough - @len should be replaced with @pcrf->len. >> >> The strict matching has to account for the length of current PCI >> config register access. We have to specify precise region through >> @start and @len in pci_virt_add_filter() when creating the filter. >> We lost flexibility. This changes the strict criteria to a relaxed >> one - the matched filter is returned when the accessed region is >> overlapped with the filter's region. With this change, the individual >> filter's handlers should validate the accessed (register) region if >> necessary. >> >> Fixes: 85daa7f00c50 ("core/pci: Support virtual device") >> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> > >For some reason I marked this as "Accepted" in patchwork and didn't >actually merge+push. My extreme bad. > >Anyway, merged to master as of e2fa37e75e428220940e0d901f0ad6d0cf0db37c > Thanks, Stewart. > >-- >Stewart Smith >OPAL Architect, IBM.
diff --git a/core/pci-virt.c b/core/pci-virt.c index 570c6e8..b531470 100644 --- a/core/pci-virt.c +++ b/core/pci-virt.c @@ -55,9 +55,14 @@ static struct pci_cfg_reg_filter *pci_virt_find_filter( if (!pvd || !len || start >= pvd->cfg_size) return NULL; + /* Return filter if there is overlapped region. We don't + * require strict matching for more flexibility. It also + * means the associated handler should validate the register + * offset and length. + */ list_for_each(&pvd->pcrf, pcrf, link) { - if (start >= pcrf->start && - (start + len) <= (pcrf->start + len)) + if (start < (pcrf->start + pcrf->len) && + (start + len) > pcrf->start) return pcrf; }
The criteria to match the filter according to PCI config register offset and length is strict one, meaning the accessed region should matches with the filter's tracking region. However, the code isn't correct enough - @len should be replaced with @pcrf->len. The strict matching has to account for the length of current PCI config register access. We have to specify precise region through @start and @len in pci_virt_add_filter() when creating the filter. We lost flexibility. This changes the strict criteria to a relaxed one - the matched filter is returned when the accessed region is overlapped with the filter's region. With this change, the individual filter's handlers should validate the accessed (register) region if necessary. Fixes: 85daa7f00c50 ("core/pci: Support virtual device") Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> --- core/pci-virt.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)