All of lore.kernel.org
 help / color / mirror / Atom feed
* 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
* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.