| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 19 Dec 2014 16:13:29 +0100 (CET)
From: Sebastian Ott <sebott@...ux.vnet.ibm.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
cc: linux-kernel@...r.kernel.org,
Rusty Russell <rusty@...tcorp.com.au>,
virtualization@...ts.linux-foundation.org,
linux-sh@...r.kernel.org, Cornelia Huck <cornelia.huck@...ibm.com>,
Martin Schwidefsky <schwidefsky@...ibm.com>,
Heiko Carstens <heiko.carstens@...ibm.com>,
linux390@...ibm.com, Gerald Schaefer <gerald.schaefer@...ibm.com>,
linux-s390@...r.kernel.org
Subject: Re: [PATCH RFC 2/5] s390: add pci_iomap_range
On Fri, 19 Dec 2014, Michael S. Tsirkin wrote:
> On Fri, Dec 19, 2014 at 03:13:37PM +0100, Sebastian Ott wrote:
> > On Mon, 15 Dec 2014, Michael S. Tsirkin wrote:
> > > From: Michael S Tsirkin <mst@...hat.com>
> > >
> > > Virtio drivers should map the part of the range they need, not
> > > necessarily all of it.
> > > To this end, support mapping ranges within BAR on s390.
> > > Since multiple ranges can now be mapped within a BAR, we keep track of
> > > the number of mappings created, and only clear out the mapping for a BAR
> > > when this number reaches 0.
> > >
> >
> > I can't say much about the users of this interface but in principle I'm
> > OK with such a change.
>
> I don't have an s390 system with pci for testing - could you help me out
> by testing this and confirming it doesn't break things?
I did some minor tests and nothing broke...
>
> > > Signed-off-by: Michael S. Tsirkin <mst@...hat.com>
> > > ---
> > > arch/s390/include/asm/pci_io.h | 1 +
> > > arch/s390/pci/pci.c | 34 +++++++++++++++++++++++++++-------
> > > 2 files changed, 28 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/arch/s390/include/asm/pci_io.h b/arch/s390/include/asm/pci_io.h
> > > index d194d54..25228b3 100644
> > > --- a/arch/s390/include/asm/pci_io.h
> > > +++ b/arch/s390/include/asm/pci_io.h
> > > @@ -16,6 +16,7 @@
> > > struct zpci_iomap_entry {
> > > u32 fh;
> > > u8 bar;
> > > + u16 count;
> > > };
> > >
> > > extern struct zpci_iomap_entry *zpci_iomap_start;
> > > diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> > > index 2fa7b14..51cb653 100644
> > > --- a/arch/s390/pci/pci.c
> > > +++ b/arch/s390/pci/pci.c
> > > @@ -259,7 +259,10 @@ void __iowrite64_copy(void __iomem *to, const void *from, size_t count)
> > > }
> > >
> > > /* Create a virtual mapping cookie for a PCI BAR */
> > > -void __iomem *pci_iomap(struct pci_dev *pdev, int bar, unsigned long max)
> > > +void __iomem *pci_iomap_range(struct pci_dev *pdev,
> > > + int bar,
> > > + unsigned long offset,
> > > + unsigned long max)
> > > {
> > > struct zpci_dev *zdev = get_zdev(pdev);
> > > u64 addr;
> > > @@ -270,14 +273,27 @@ void __iomem *pci_iomap(struct pci_dev *pdev, int bar, unsigned long max)
> > >
> > > idx = zdev->bars[bar].map_idx;
> > > spin_lock(&zpci_iomap_lock);
> > > - zpci_iomap_start[idx].fh = zdev->fh;
> > > - zpci_iomap_start[idx].bar = bar;
> > > + if (zpci_iomap_start[idx].count++) {
> > > + BUG_ON(zpci_iomap_start[idx].fh != zdev->fh ||
> > > + zpci_iomap_start[idx].bar != bar);
> > > + } else {
> > > + zpci_iomap_start[idx].fh = zdev->fh;
> > > + zpci_iomap_start[idx].bar = bar;
> > > + }
> > > + /* Detect overrun */
> > > + BUG_ON(!zpci_iomap_start[idx].count);
> > > spin_unlock(&zpci_iomap_lock);
> > >
> > > addr = ZPCI_IOMAP_ADDR_BASE | ((u64) idx << 48);
> > > - return (void __iomem *) addr;
> > > + return (void __iomem *) addr + offset;
> > > }
> > > -EXPORT_SYMBOL_GPL(pci_iomap);
> > > +EXPORT_SYMBOL_GPL(pci_iomap_range);
> > > +
> > > +void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
> > > +{
> > > + return pci_iomap_range(dev, bar, 0, maxlen);
> > > +}
> > > +EXPORT_SYMBOL(pci_iomap);
> >
> > This was EXPORT_SYMBOL_GPL. I guess, for this patch, it should stay that
> > way. ...Hm, everyone else has this stuff as EXPORT_SYMBOL looks like we
> > should use that too.
> >
> > Regards,
> > Sebastian
>
> OK, so you want two patches: one with new functionality,
> one switching to EXPORT_SYMBOL?
Either that or at least add a note in the patch description.
Regards,
Sebastian
>
>
> > >
> > > void pci_iounmap(struct pci_dev *pdev, void __iomem *addr)
> > > {
> > > @@ -285,8 +301,12 @@ void pci_iounmap(struct pci_dev *pdev, void __iomem *addr)
> > >
> > > idx = (((__force u64) addr) & ~ZPCI_IOMAP_ADDR_BASE) >> 48;
> > > spin_lock(&zpci_iomap_lock);
> > > - zpci_iomap_start[idx].fh = 0;
> > > - zpci_iomap_start[idx].bar = 0;
> > > + /* Detect underrun */
> > > + BUG_ON(!zpci_iomap_start[idx].count);
> > > + if (!--zpci_iomap_start[idx].count) {
> > > + zpci_iomap_start[idx].fh = 0;
> > > + zpci_iomap_start[idx].bar = 0;
> > > + }
> > > spin_unlock(&zpci_iomap_lock);
> > > }
> > > EXPORT_SYMBOL_GPL(pci_iounmap);
> > > --
> > > MST
> > >
> > >
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists