* PAGE_KERNEL_RO @ 2011-01-13 0:07 Andrew Morton 2011-01-13 0:11 ` PAGE_KERNEL_RO Benjamin Herrenschmidt 0 siblings, 1 reply; 6+ messages in thread From: Andrew Morton @ 2011-01-13 0:07 UTC (permalink / raw) To: linux-arch; +Cc: Stefan Richter drivers/firewire/ohci.c now needs PAGE_KERNEL_RO, but many architectures don't implement it. Broke my sparc64 build. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: PAGE_KERNEL_RO 2011-01-13 0:07 PAGE_KERNEL_RO Andrew Morton @ 2011-01-13 0:11 ` Benjamin Herrenschmidt 2011-01-13 6:34 ` PAGE_KERNEL_RO Stefan Richter 0 siblings, 1 reply; 6+ messages in thread From: Benjamin Herrenschmidt @ 2011-01-13 0:11 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-arch, Stefan Richter On Wed, 2011-01-12 at 16:07 -0800, Andrew Morton wrote: > drivers/firewire/ohci.c now needs PAGE_KERNEL_RO, but many > architectures don't implement it. Broke my sparc64 build. Some architectures actually cannot implement it even... at least some variants of powerpc MMUs don't have a combination of protection bits that allow a kernel-only RO mapping (yeah odd). Cheers, Ben. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: PAGE_KERNEL_RO 2011-01-13 0:11 ` PAGE_KERNEL_RO Benjamin Herrenschmidt @ 2011-01-13 6:34 ` Stefan Richter [not found] ` <522C1DF17AF50042AD8AE87F7887BD3D01E1655529@exch.hq.tensilica.com> 0 siblings, 1 reply; 6+ messages in thread From: Stefan Richter @ 2011-01-13 6:34 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Andrew Morton, linux-arch, linux1394-devel, Clemens Ladisch On Jan 13 Benjamin Herrenschmidt wrote at linux-arch: > On Wed, 2011-01-12 at 16:07 -0800, Andrew Morton wrote: > > drivers/firewire/ohci.c now needs PAGE_KERNEL_RO, but many > > architectures don't implement it. Broke my sparc64 build. > > Some architectures actually cannot implement it even... at least some > variants of powerpc MMUs don't have a combination of protection bits > that allow a kernel-only RO mapping (yeah odd). The simplest perceivable fix, to disable firewire-ohci on architectures which don't have PAGE_KERNEL_RO, would be bad since there are actually sparc64 machines with these controllers. As far as I can tell, the new RO mapping in firewire-ohci can as well be an r/w mapping. We just never need to write at these virtual addresses. So, should we just change the driver to map it r/w when we can't have PAGE_KERNEL_RO, or for simplicity on all architectures? -- Stefan Richter -=====-==-== ---= -==-= http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <522C1DF17AF50042AD8AE87F7887BD3D01E1655529@exch.hq.tensilica.com>]
* Re: PAGE_KERNEL_RO [not found] ` <522C1DF17AF50042AD8AE87F7887BD3D01E1655529@exch.hq.tensilica.com> @ 2011-01-13 7:27 ` Clemens Ladisch 2011-01-13 9:04 ` PAGE_KERNEL_RO James Bottomley 0 siblings, 1 reply; 6+ messages in thread From: Clemens Ladisch @ 2011-01-13 7:27 UTC (permalink / raw) To: Marc Gauthier, Stefan Richter Cc: Benjamin Herrenschmidt, Andrew Morton, linux-arch, linux1394-devel Marc Gauthier wrote: > Stefan Richter wrote: >> On Jan 13 Benjamin Herrenschmidt wrote at linux-arch: >>> On Wed, 2011-01-12 at 16:07 -0800, Andrew Morton wrote: >>>> drivers/firewire/ohci.c now needs PAGE_KERNEL_RO, but many >>>> architectures don't implement it. Broke my sparc64 build. >>> >>> Some architectures actually cannot implement it even... at least some >>> variants of powerpc MMUs don't have a combination of protection bits >>> that allow a kernel-only RO mapping (yeah odd). >> >> The simplest perceivable fix, to disable firewire-ohci on >> architectures which don't have PAGE_KERNEL_RO, would be bad since >> there are actually sparc64 machines with these controllers. >> >> As far as I can tell, the new RO mapping in firewire-ohci can as well be >> an r/w mapping. We just never need to write at these virtual >> addresses. So, should we just change the driver to map it r/w when we >> can't have PAGE_KERNEL_RO, or for simplicity on all architectures? > > What do I know... Is a simple: > > #ifndef PAGE_KERNEL_RO > #define PAGE_KERNEL_RO PAGE_KERNEL > #endif > > in drivers/firewire/ohci.c too hacky? --8<---------------------------------------------------------------->8-- firewire: ohci: fix compilation on arches without PAGE_KERNEL_RO, e.g. sparc PAGE_KERNEL_RO is not available on all architectures, so its use in the new AR code broke compilation on sparc64. Because the R/O mapping is only used to catch drivers that try to write to the reception buffer and not actually required for correct operation, we can just use a normal PAGE_KERNEL mapping where _RO is not available. Thanks to Stefan Richter and Marc Gauthier <marc@tensilica.com> for suggesting this fix. Signed-off-by: Clemens Ladisch <clemens@ladisch.de> --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -91,6 +91,14 @@ struct descriptor { #define MAX_AR_PACKET_SIZE (16 + MAX_ASYNC_PAYLOAD + 4) #define AR_WRAPAROUND_PAGES DIV_ROUND_UP(MAX_AR_PACKET_SIZE, PAGE_SIZE) +/* + * For archs where PAGE_KERNEL_RO is not supported; + * mapping the AR buffers readonly for the CPU is just a debugging aid. + */ +#ifndef PAGE_KERNEL_RO +#define PAGE_KERNEL_RO PAGE_KERNEL +#endif + struct ar_context { struct fw_ohci *ohci; struct page *pages[AR_BUFFERS]; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: PAGE_KERNEL_RO 2011-01-13 7:27 ` PAGE_KERNEL_RO Clemens Ladisch @ 2011-01-13 9:04 ` James Bottomley 2011-01-13 9:12 ` PAGE_KERNEL_RO Clemens Ladisch 0 siblings, 1 reply; 6+ messages in thread From: James Bottomley @ 2011-01-13 9:04 UTC (permalink / raw) To: Clemens Ladisch Cc: Marc Gauthier, Stefan Richter, Benjamin Herrenschmidt, Andrew Morton, linux-arch, linux1394-devel On Thu, 2011-01-13 at 08:27 +0100, Clemens Ladisch wrote: > Marc Gauthier wrote: > > Stefan Richter wrote: > >> On Jan 13 Benjamin Herrenschmidt wrote at linux-arch: > >>> On Wed, 2011-01-12 at 16:07 -0800, Andrew Morton wrote: > >>>> drivers/firewire/ohci.c now needs PAGE_KERNEL_RO, but many > >>>> architectures don't implement it. Broke my sparc64 build. > >>> > >>> Some architectures actually cannot implement it even... at least some > >>> variants of powerpc MMUs don't have a combination of protection bits > >>> that allow a kernel-only RO mapping (yeah odd). > >> > >> The simplest perceivable fix, to disable firewire-ohci on > >> architectures which don't have PAGE_KERNEL_RO, would be bad since > >> there are actually sparc64 machines with these controllers. > >> > >> As far as I can tell, the new RO mapping in firewire-ohci can as well be > >> an r/w mapping. We just never need to write at these virtual > >> addresses. So, should we just change the driver to map it r/w when we > >> can't have PAGE_KERNEL_RO, or for simplicity on all architectures? > > > > What do I know... Is a simple: > > > > #ifndef PAGE_KERNEL_RO > > #define PAGE_KERNEL_RO PAGE_KERNEL > > #endif > > > > in drivers/firewire/ohci.c too hacky? > > --8<---------------------------------------------------------------->8-- > firewire: ohci: fix compilation on arches without PAGE_KERNEL_RO, e.g. sparc > > PAGE_KERNEL_RO is not available on all architectures, so its use in the > new AR code broke compilation on sparc64. > > Because the R/O mapping is only used to catch drivers that try to write > to the reception buffer and not actually required for correct operation, > we can just use a normal PAGE_KERNEL mapping where _RO is not available. > > Thanks to Stefan Richter and Marc Gauthier <marc@tensilica.com> for > suggesting this fix. > > Signed-off-by: Clemens Ladisch <clemens@ladisch.de> > > --- a/drivers/firewire/ohci.c > +++ b/drivers/firewire/ohci.c > @@ -91,6 +91,14 @@ struct descriptor { > #define MAX_AR_PACKET_SIZE (16 + MAX_ASYNC_PAYLOAD + 4) > #define AR_WRAPAROUND_PAGES DIV_ROUND_UP(MAX_AR_PACKET_SIZE, PAGE_SIZE) > > +/* > + * For archs where PAGE_KERNEL_RO is not supported; > + * mapping the AR buffers readonly for the CPU is just a debugging aid. > + */ > +#ifndef PAGE_KERNEL_RO > +#define PAGE_KERNEL_RO PAGE_KERNEL > +#endif This might cause interesting issues on sparc64 if it ever acquired a PAGE_KERNEL_RO. Sparc64 has extern pgprot_t for it's PAGE_KERNEL types rather than #defines, so the #ifdef check wouldn't see this. I think either PAGE_PROT_RO becomes part of our arch API (so all architectures are forced to add it), or, if it's not part of the API, ohci isn't entitled to use it. The latter seems simplest since you have no real use for write protection anyway. James ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: PAGE_KERNEL_RO 2011-01-13 9:04 ` PAGE_KERNEL_RO James Bottomley @ 2011-01-13 9:12 ` Clemens Ladisch 0 siblings, 0 replies; 6+ messages in thread From: Clemens Ladisch @ 2011-01-13 9:12 UTC (permalink / raw) To: James Bottomley, Stefan Richter Cc: Marc Gauthier, Benjamin Herrenschmidt, Andrew Morton, linux-arch, linux1394-devel James Bottomley wrote: > On Thu, 2011-01-13 at 08:27 +0100, Clemens Ladisch wrote: >> firewire: ohci: fix compilation on arches without PAGE_KERNEL_RO, e.g. sparc >> >> PAGE_KERNEL_RO is not available on all architectures, so its use in the >> new AR code broke compilation on sparc64. >> >> Because the R/O mapping is only used to catch drivers that try to write >> to the reception buffer and not actually required for correct operation, >> we can just use a normal PAGE_KERNEL mapping where _RO is not available. >> >> Thanks to Stefan Richter and Marc Gauthier <marc@tensilica.com> for >> suggesting this fix. >> >> Signed-off-by: Clemens Ladisch <clemens@ladisch.de> >> >> --- a/drivers/firewire/ohci.c >> +++ b/drivers/firewire/ohci.c >> @@ -91,6 +91,14 @@ struct descriptor { >> #define MAX_AR_PACKET_SIZE (16 + MAX_ASYNC_PAYLOAD + 4) >> #define AR_WRAPAROUND_PAGES DIV_ROUND_UP(MAX_AR_PACKET_SIZE, PAGE_SIZE) >> >> +/* >> + * For archs where PAGE_KERNEL_RO is not supported; >> + * mapping the AR buffers readonly for the CPU is just a debugging aid. >> + */ >> +#ifndef PAGE_KERNEL_RO >> +#define PAGE_KERNEL_RO PAGE_KERNEL >> +#endif > > This might cause interesting issues on sparc64 if it ever acquired a > PAGE_KERNEL_RO. Sparc64 has extern pgprot_t for it's PAGE_KERNEL types > rather than #defines, so the #ifdef check wouldn't see this. > > I think either PAGE_PROT_RO becomes part of our arch API (so all > architectures are forced to add it), or, if it's not part of the API, > ohci isn't entitled to use it. The latter seems simplest since you have > no real use for write protection anyway. --8<---------------------------------------------------------------->8-- firewire: ohci: fix compilation on arches without PAGE_KERNEL_RO PAGE_KERNEL_RO is not available on all architectures, so its use in the new AR code broke compilation on sparc64. Because the read-only mapping was just a debugging aid, just use PAGE_KERNEL instead. Signed-off-by: Clemens Ladisch <clemens@ladisch.de> --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -961,7 +961,7 @@ static int ar_context_init(struct ar_context *ctx, struct fw_ohci *ohci, for (i = 0; i < AR_WRAPAROUND_PAGES; i++) pages[AR_BUFFERS + i] = ctx->pages[i]; ctx->buffer = vm_map_ram(pages, AR_BUFFERS + AR_WRAPAROUND_PAGES, - -1, PAGE_KERNEL_RO); + -1, PAGE_KERNEL); if (!ctx->buffer) goto out_of_memory; ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-01-13 9:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-13 0:07 PAGE_KERNEL_RO Andrew Morton
2011-01-13 0:11 ` PAGE_KERNEL_RO Benjamin Herrenschmidt
2011-01-13 6:34 ` PAGE_KERNEL_RO Stefan Richter
[not found] ` <522C1DF17AF50042AD8AE87F7887BD3D01E1655529@exch.hq.tensilica.com>
2011-01-13 7:27 ` PAGE_KERNEL_RO Clemens Ladisch
2011-01-13 9:04 ` PAGE_KERNEL_RO James Bottomley
2011-01-13 9:12 ` PAGE_KERNEL_RO Clemens Ladisch
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).