user-mode-linux-user

Re: [uml-user] [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

[Chen G. <gan...@as...>]

On 06/26/2013 05:38 PM, Richard Weinberger wrote:
> Am 26.06.2013 11:33, schrieb Chen Gang:
>> > On 06/26/2013 05:03 PM, Richard Weinberger wrote:
>>> >> Am 26.06.2013 10:58, schrieb Chen Gang:
>>>>> >>>> On 06/26/2013 04:39 PM, Richard Weinberger wrote:
>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> The drivers internal code has already check the related return value,
>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> so it is the architecture's duty to 'tell' the driver whether support
>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> io memory (e.g. define ioremap, but return NULL).
>>>>>>>>>>>>>>> >>>>>>>>>>>>>> It does so already by setting CONFIG_HAS_IOMEM=n
>>>>>>>>>>> >>>>>>>>>>
>>>>>>>>>>> >>>>>>>>>> Excuse me, I use "grep -rn ioremap *" under "include/" and "arch/um/"
>>>>>>>>>>> >>>>>>>>>> directory, but can not find the related definition for 'ioremap'.
>>>>>>>>>>> >>>>>>>>>>
>>>>>>>>>>> >>>>>>>>>> Is there another declaration or definition way which I don't know ?
>>>>>>>>>>> >>>>>>>>>> (maybe it is).
>>>>>>> >>>>>> Both UML and s390 (in the !CONFIG_PCI) do not define ioremap() because
>>>>>>> >>>>>> without io memory you cannot have a ioremap().
>>>>>>> >>>>>>
>>>>> >>>>
>>>>> >>>> I assume if ioremap() return NULL, it means "without io memory", is it
>>>>> >>>> correct ?
>>>>> >>>>
>>>>> >>>> If it is correct, "define a dummy ioremap(), and return NULL" is just
>>>>> >>>> the meaning that you mentioned above.
>>>>> >>>>
>>>>> >>>> If so, for UML, it is not requirement, but recommend to define a dummy
>>>>> >>>> ioremap() which return NULL, so can be generic enough to mach all cases.
>>> >> No.
>>> >> Not setting CONFIG_HAS_IOMEM=y means "This arch has no io memory and therefore no
>>> >> functions to mess with it".
>>> >>
>> > 
>> > Since the API itself already contents the meaning: "return NULL means
>> > the arch has no related io memory",
>> > 
>> > Why not define a generic dummy one in "include/asm-generic/io.h" instead
>> > of "HAS_IOMEM" (which has already spread many various places, and also,
>> > most of new drivers have to know about it).
>> > 
>> > e.g: in "include/asm-generic/io.h", if "CONFIG_HAS_IOMEM=n", define a
>> > dummy ioremap() which return NULL ... (also need consider more details).
> Because we don't even want to build these drivers and not make them fail while
> executing io memory related functions.
> 

It is one reason, but it seems not quite enough for it, it depends on
our requirements.

If we are also the 'platform' of all kernel modules (not only for user
mode), and "include/asm-generic/*" plays an important role for it.

It is the modules their own choice to determine which architectures they
want to support, and which architecture they won't, not need be directed
or forced by 'platform'.

Also better to let kernel modules free to determine whether build in or
not, when actually do not support the specific features of architecture
currently, not need be directed or forced by 'platform'.


Thanks.
-- 
Chen Gang

Asianux Corporation

Re: [uml-user] [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

[Geert U. <ge...@li...>]

On Wed, Jun 26, 2013 at 11:38 AM, Richard Weinberger <ri...@no...> wrote:
>> Since the API itself already contents the meaning: "return NULL means
>> the arch has no related io memory",

No, NULL means it could not map the I/O memory.

>> Why not define a generic dummy one in "include/asm-generic/io.h" instead
>> of "HAS_IOMEM" (which has already spread many various places, and also,
>> most of new drivers have to know about it).
>>
>> e.g: in "include/asm-generic/io.h", if "CONFIG_HAS_IOMEM=n", define a
>> dummy ioremap() which return NULL ... (also need consider more details).
>
> Because we don't even want to build these drivers and not make them fail while
> executing io memory related functions.

Indeed, it doesn't make sense to build drivers that cannot work.
And they may fail in a very bad way.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@li...

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [uml-user] [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

[Richard W. <ri...@no...>]

Am 26.06.2013 11:33, schrieb Chen Gang:
> On 06/26/2013 05:03 PM, Richard Weinberger wrote:
>> Am 26.06.2013 10:58, schrieb Chen Gang:
>>>> On 06/26/2013 04:39 PM, Richard Weinberger wrote:
>>>>>>>>>>>>>>>>>>>>>> The drivers internal code has already check the related return value,
>>>>>>>>>>>>>>>>>>>>>> so it is the architecture's duty to 'tell' the driver whether support
>>>>>>>>>>>>>>>>>>>>>> io memory (e.g. define ioremap, but return NULL).
>>>>>>>>>>>>>> It does so already by setting CONFIG_HAS_IOMEM=n
>>>>>>>>>>
>>>>>>>>>> Excuse me, I use "grep -rn ioremap *" under "include/" and "arch/um/"
>>>>>>>>>> directory, but can not find the related definition for 'ioremap'.
>>>>>>>>>>
>>>>>>>>>> Is there another declaration or definition way which I don't know ?
>>>>>>>>>> (maybe it is).
>>>>>> Both UML and s390 (in the !CONFIG_PCI) do not define ioremap() because
>>>>>> without io memory you cannot have a ioremap().
>>>>>>
>>>>
>>>> I assume if ioremap() return NULL, it means "without io memory", is it
>>>> correct ?
>>>>
>>>> If it is correct, "define a dummy ioremap(), and return NULL" is just
>>>> the meaning that you mentioned above.
>>>>
>>>> If so, for UML, it is not requirement, but recommend to define a dummy
>>>> ioremap() which return NULL, so can be generic enough to mach all cases.
>> No.
>> Not setting CONFIG_HAS_IOMEM=y means "This arch has no io memory and therefore no
>> functions to mess with it".
>>
> 
> Since the API itself already contents the meaning: "return NULL means
> the arch has no related io memory",
> 
> Why not define a generic dummy one in "include/asm-generic/io.h" instead
> of "HAS_IOMEM" (which has already spread many various places, and also,
> most of new drivers have to know about it).
> 
> e.g: in "include/asm-generic/io.h", if "CONFIG_HAS_IOMEM=n", define a
> dummy ioremap() which return NULL ... (also need consider more details).

Because we don't even want to build these drivers and not make them fail while
executing io memory related functions.

> 
> All together, I think: it is the duty of "asm-generic/io.h" to process
> this issue, not the duty of many drivers and some architectures.
> 
> 
>> Let's get back to the real problem,
>> drivers/ptp/ptp_pch.c does not build on UML (and I'm very sure also not on S390).
>> Fix the issue by making it depend on HAS_IOMEM.
>>
> 
> At least now, it seems it is the only suitable way to fix this issue.  :-(
> 
> 
>> Btw: Did you actually look at this driver? There is *zero* reason to have it on UML.
>> ..like 99.9% of all other drivers which use io memory.
> 
> Excuse me, I did not look at the details of this driver. Maybe it just
> like you said above.

It is.

Thanks,
//richard

Re: [uml-user] [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

[Chen G. <gan...@as...>]

On 06/26/2013 05:03 PM, Richard Weinberger wrote:
> Am 26.06.2013 10:58, schrieb Chen Gang:
>> > On 06/26/2013 04:39 PM, Richard Weinberger wrote:
>>>>>>>>>>> >>>>>>>>>> The drivers internal code has already check the related return value,
>>>>>>>>>>> >>>>>>>>>> so it is the architecture's duty to 'tell' the driver whether support
>>>>>>>>>>> >>>>>>>>>> io memory (e.g. define ioremap, but return NULL).
>>>>>>> >>>>>> It does so already by setting CONFIG_HAS_IOMEM=n
>>>>> >>>>
>>>>> >>>> Excuse me, I use "grep -rn ioremap *" under "include/" and "arch/um/"
>>>>> >>>> directory, but can not find the related definition for 'ioremap'.
>>>>> >>>>
>>>>> >>>> Is there another declaration or definition way which I don't know ?
>>>>> >>>> (maybe it is).
>>> >> Both UML and s390 (in the !CONFIG_PCI) do not define ioremap() because
>>> >> without io memory you cannot have a ioremap().
>>> >>
>> > 
>> > I assume if ioremap() return NULL, it means "without io memory", is it
>> > correct ?
>> > 
>> > If it is correct, "define a dummy ioremap(), and return NULL" is just
>> > the meaning that you mentioned above.
>> > 
>> > If so, for UML, it is not requirement, but recommend to define a dummy
>> > ioremap() which return NULL, so can be generic enough to mach all cases.
> No.
> Not setting CONFIG_HAS_IOMEM=y means "This arch has no io memory and therefore no
> functions to mess with it".
> 

Since the API itself already contents the meaning: "return NULL means
the arch has no related io memory",

Why not define a generic dummy one in "include/asm-generic/io.h" instead
of "HAS_IOMEM" (which has already spread many various places, and also,
most of new drivers have to know about it).

e.g: in "include/asm-generic/io.h", if "CONFIG_HAS_IOMEM=n", define a
dummy ioremap() which return NULL ... (also need consider more details).


All together, I think: it is the duty of "asm-generic/io.h" to process
this issue, not the duty of many drivers and some architectures.


> Let's get back to the real problem,
> drivers/ptp/ptp_pch.c does not build on UML (and I'm very sure also not on S390).
> Fix the issue by making it depend on HAS_IOMEM.
> 

At least now, it seems it is the only suitable way to fix this issue.  :-(


> Btw: Did you actually look at this driver? There is *zero* reason to have it on UML.
> ..like 99.9% of all other drivers which use io memory.

Excuse me, I did not look at the details of this driver. Maybe it just
like you said above.


Thanks.
-- 
Chen Gang

Asianux Corporation

Re: [uml-user] [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

[Richard W. <ri...@no...>]

Am 26.06.2013 10:58, schrieb Chen Gang:
> On 06/26/2013 04:39 PM, Richard Weinberger wrote:
>>>>>>>>>> The drivers internal code has already check the related return value,
>>>>>>>>>> so it is the architecture's duty to 'tell' the driver whether support
>>>>>>>>>> io memory (e.g. define ioremap, but return NULL).
>>>>>> It does so already by setting CONFIG_HAS_IOMEM=n
>>>>
>>>> Excuse me, I use "grep -rn ioremap *" under "include/" and "arch/um/"
>>>> directory, but can not find the related definition for 'ioremap'.
>>>>
>>>> Is there another declaration or definition way which I don't know ?
>>>> (maybe it is).
>> Both UML and s390 (in the !CONFIG_PCI) do not define ioremap() because
>> without io memory you cannot have a ioremap().
>>
> 
> I assume if ioremap() return NULL, it means "without io memory", is it
> correct ?
> 
> If it is correct, "define a dummy ioremap(), and return NULL" is just
> the meaning that you mentioned above.
> 
> If so, for UML, it is not requirement, but recommend to define a dummy
> ioremap() which return NULL, so can be generic enough to mach all cases.

No.
Not setting CONFIG_HAS_IOMEM=y means "This arch has no io memory and therefore no
functions to mess with it".

Let's get back to the real problem,
drivers/ptp/ptp_pch.c does not build on UML (and I'm very sure also not on S390).
Fix the issue by making it depend on HAS_IOMEM.

Btw: Did you actually look at this driver? There is *zero* reason to have it on UML.
...like 99.9% of all other drivers which use io memory.

Thanks,
//richard

Re: [uml-user] [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

[Chen G. <gan...@as...>]

On 06/26/2013 04:39 PM, Richard Weinberger wrote:
>>>>> >>>> The drivers internal code has already check the related return value,
>>>>> >>>> so it is the architecture's duty to 'tell' the driver whether support
>>>>> >>>> io memory (e.g. define ioremap, but return NULL).
>>> >> It does so already by setting CONFIG_HAS_IOMEM=n
>> > 
>> > Excuse me, I use "grep -rn ioremap *" under "include/" and "arch/um/"
>> > directory, but can not find the related definition for 'ioremap'.
>> > 
>> > Is there another declaration or definition way which I don't know ?
>> > (maybe it is).
> Both UML and s390 (in the !CONFIG_PCI) do not define ioremap() because
> without io memory you cannot have a ioremap().
> 

I assume if ioremap() return NULL, it means "without io memory", is it
correct ?

If it is correct, "define a dummy ioremap(), and return NULL" is just
the meaning that you mentioned above.

If so, for UML, it is not requirement, but recommend to define a dummy
ioremap() which return NULL, so can be generic enough to mach all cases.


>> > For our case, the ".config" file does not define 'CONFIG_HAS_IOMEM', can
>> > I assume it means "CONFIG_HAS_IOMEM=n" ?
> If I'm not mistaken it works the other way around.
> All archs except UML and s390 set CONFIG_HAS_IOMEM=y.

I guess so, too.


Thanks.
-- 
Chen Gang

Asianux Corporation

Re: [uml-user] [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

[Richard W. <ri...@no...>]

Am 26.06.2013 10:34, schrieb Chen Gang:
> On 06/26/2013 04:05 PM, Richard Weinberger wrote:
>>>>>>>> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
>>>>>>>> index d5afe96..e80331d 100644
>>>>>>>> --- a/include/asm-generic/io.h
>>>>>>>> +++ b/include/asm-generic/io.h
>>>>>>>> @@ -303,10 +303,10 @@ static inline void *phys_to_virt(unsigned long address)
>>>>>>>>  /*
>>>>>>>>   * Change "struct page" to physical address.
>>>>>>>>   *
>>>>>>>> - * This implementation is for the no-MMU case only... if you have an MMU
>>>>>>>> + * This implementation is for the no-MMU or UML case only... if you have an MMU
>>>>>>>>   * you'll need to provide your own definitions.
>>>>>>>>   */
>>>>>>>> -#ifndef CONFIG_MMU
>>>>>>>> +#if !CONFIG_MMU || CONFIG_UML
>>>>>>>>  static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
>>>>>>>>  {
>>>>>>>>  	return (void __iomem*) (unsigned long)offset;
>>>>>>>> @@ -325,7 +325,7 @@ static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
>>>>>>>>  static inline void iounmap(void __iomem *addr)
>>>>>>>>  {
>>>>>>>>  }
>>>>>>>> -#endif /* CONFIG_MMU */
>>>>>>>> +#endif /* !CONFIG_MMU || CONFIG_UML */
>>>>>>>>  
>>>>>>>>  #ifdef CONFIG_HAS_IOPORT
>>>>>>>>  #ifndef CONFIG_GENERIC_IOMAP
>>>>>>>>
>>>>>>
>>>>>> UML has no io memory but a MMU, so I'd argue that you better fix drivers/ptp/ptp_pch.c dependencies.
>>>>>> _If_ ptp_pch.c really works without real io memory, you can look what I did in my GENERIC_IO series[1]
>>>>>> to make nandsim work on UML. Maybe this helps.
>>>>>>
>>>>
>>>> But "no io memory" is not the excuse to not define the related dummy
>>>> function.
>> UML has no io memory, period.
>> Same applies for s390, it also includes asm-generic/io.h in the !CONFIG_PCI
>> case.
>> UML and s390 are very special here.
>>
> 
> Oh, yes, really the same.
> 
>>>> The drivers internal code has already check the related return value,
>>>> so it is the architecture's duty to 'tell' the driver whether support
>>>> io memory (e.g. define ioremap, but return NULL).
>> It does so already by setting CONFIG_HAS_IOMEM=n
> 
> Excuse me, I use "grep -rn ioremap *" under "include/" and "arch/um/"
> directory, but can not find the related definition for 'ioremap'.
> 
> Is there another declaration or definition way which I don't know ?
> (maybe it is).

Both UML and s390 (in the !CONFIG_PCI) do not define ioremap() because
without io memory you cannot have a ioremap().

> For our case, the ".config" file does not define 'CONFIG_HAS_IOMEM', can
> I assume it means "CONFIG_HAS_IOMEM=n" ?

If I'm not mistaken it works the other way around.
All archs except UML and s390 set CONFIG_HAS_IOMEM=y.

Thanks,
//richard

Re: [uml-user] [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

[Chen G. <gan...@as...>]

On 06/26/2013 04:05 PM, Richard Weinberger wrote:
>>>> >>> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
>>>> >>> index d5afe96..e80331d 100644
>>>> >>> --- a/include/asm-generic/io.h
>>>> >>> +++ b/include/asm-generic/io.h
>>>> >>> @@ -303,10 +303,10 @@ static inline void *phys_to_virt(unsigned long address)
>>>> >>>  /*
>>>> >>>   * Change "struct page" to physical address.
>>>> >>>   *
>>>> >>> - * This implementation is for the no-MMU case only... if you have an MMU
>>>> >>> + * This implementation is for the no-MMU or UML case only... if you have an MMU
>>>> >>>   * you'll need to provide your own definitions.
>>>> >>>   */
>>>> >>> -#ifndef CONFIG_MMU
>>>> >>> +#if !CONFIG_MMU || CONFIG_UML
>>>> >>>  static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
>>>> >>>  {
>>>> >>>  	return (void __iomem*) (unsigned long)offset;
>>>> >>> @@ -325,7 +325,7 @@ static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
>>>> >>>  static inline void iounmap(void __iomem *addr)
>>>> >>>  {
>>>> >>>  }
>>>> >>> -#endif /* CONFIG_MMU */
>>>> >>> +#endif /* !CONFIG_MMU || CONFIG_UML */
>>>> >>>  
>>>> >>>  #ifdef CONFIG_HAS_IOPORT
>>>> >>>  #ifndef CONFIG_GENERIC_IOMAP
>>>> >>>
>>> >>
>>> >> UML has no io memory but a MMU, so I'd argue that you better fix drivers/ptp/ptp_pch.c dependencies.
>>> >> _If_ ptp_pch.c really works without real io memory, you can look what I did in my GENERIC_IO series[1]
>>> >> to make nandsim work on UML. Maybe this helps.
>>> >>
>> > 
>> > But "no io memory" is not the excuse to not define the related dummy
>> > function.
> UML has no io memory, period.
> Same applies for s390, it also includes asm-generic/io.h in the !CONFIG_PCI
> case.
> UML and s390 are very special here.
> 

Oh, yes, really the same.

>> > The drivers internal code has already check the related return value,
>> > so it is the architecture's duty to 'tell' the driver whether support
>> > io memory (e.g. define ioremap, but return NULL).
> It does so already by setting CONFIG_HAS_IOMEM=n

Excuse me, I use "grep -rn ioremap *" under "include/" and "arch/um/"
directory, but can not find the related definition for 'ioremap'.

Is there another declaration or definition way which I don't know ?
(maybe it is).

For our case, the ".config" file does not define 'CONFIG_HAS_IOMEM', can
I assume it means "CONFIG_HAS_IOMEM=n" ?


Thanks
-- 
Chen Gang

Asianux Corporation

Re: [uml-user] [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

[Richard W. <ri...@no...>]

Hi!

Am 26.06.2013 09:56, schrieb Chen Gang:
> On 06/26/2013 02:54 PM, Richard Weinberger wrote:
>> Hi!
>>
>> Am 26.06.2013 08:31, schrieb Chen Gang:
>>> For "User Mode Linux", it may enable 'MMU', but not need implement
>>> ioremap and iounmap, so "include/asm-generic/io.h" need notice this
>>> case to keep itself 'generic'.
>>>
>>> The related error (with allmodconfig, without pcap):
>>>
>>>     CC [M]  drivers/ptp/ptp_pch.o
>>>   drivers/ptp/ptp_pch.c: In function �pch_remove�:
>>>   drivers/ptp/ptp_pch.c:571:3: error: implicit declaration of function �iounmap� [-Werror=implicit-function-declaration]
>>>   drivers/ptp/ptp_pch.c: In function �pch_probe�:
>>>   drivers/ptp/ptp_pch.c:621:2: error: implicit declaration of function �ioremap� [-Werror=implicit-function-declaration]
>>>   drivers/ptp/ptp_pch.c:621:13: warning: assignment makes pointer from integer without a cast [enabled by default]
>>>   cc1: some warnings being treated as errors
>>>
>>>
>>> Signed-off-by: Chen Gang <gan...@as...>
>>> ---
>>>  arch/um/include/asm/Kbuild |    1 +
>>>  include/asm-generic/io.h   |    6 +++---
>>>  2 files changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/um/include/asm/Kbuild b/arch/um/include/asm/Kbuild
>>> index b30f34a..a34ea5d 100644
>>> --- a/arch/um/include/asm/Kbuild
>>> +++ b/arch/um/include/asm/Kbuild
>>> @@ -3,3 +3,4 @@ generic-y += hw_irq.h irq_regs.h kdebug.h percpu.h sections.h topology.h xor.h
>>>  generic-y += ftrace.h pci.h io.h param.h delay.h mutex.h current.h exec.h
>>>  generic-y += switch_to.h clkdev.h
>>>  generic-y += trace_clock.h
>>> +generic-y += io.h
>>
>> We include that file already. See three lines above.
>>
> 
> Oh, really it is, thanks.
> 
>>> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
>>> index d5afe96..e80331d 100644
>>> --- a/include/asm-generic/io.h
>>> +++ b/include/asm-generic/io.h
>>> @@ -303,10 +303,10 @@ static inline void *phys_to_virt(unsigned long address)
>>>  /*
>>>   * Change "struct page" to physical address.
>>>   *
>>> - * This implementation is for the no-MMU case only... if you have an MMU
>>> + * This implementation is for the no-MMU or UML case only... if you have an MMU
>>>   * you'll need to provide your own definitions.
>>>   */
>>> -#ifndef CONFIG_MMU
>>> +#if !CONFIG_MMU || CONFIG_UML
>>>  static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
>>>  {
>>>  	return (void __iomem*) (unsigned long)offset;
>>> @@ -325,7 +325,7 @@ static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
>>>  static inline void iounmap(void __iomem *addr)
>>>  {
>>>  }
>>> -#endif /* CONFIG_MMU */
>>> +#endif /* !CONFIG_MMU || CONFIG_UML */
>>>  
>>>  #ifdef CONFIG_HAS_IOPORT
>>>  #ifndef CONFIG_GENERIC_IOMAP
>>>
>>
>> UML has no io memory but a MMU, so I'd argue that you better fix drivers/ptp/ptp_pch.c dependencies.
>> _If_ ptp_pch.c really works without real io memory, you can look what I did in my GENERIC_IO series[1]
>> to make nandsim work on UML. Maybe this helps.
>>
> 
> But "no io memory" is not the excuse to not define the related dummy
> function.

UML has no io memory, period.
Same applies for s390, it also includes asm-generic/io.h in the !CONFIG_PCI
case.
UML and s390 are very special here.


> The drivers internal code has already check the related return value,
> so it is the architecture's duty to 'tell' the driver whether support
> io memory (e.g. define ioremap, but return NULL).

It does so already by setting CONFIG_HAS_IOMEM=n

Thanks,
//richard

Re: [uml-user] [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

[Chen G. <gan...@as...>]

On 06/26/2013 03:38 PM, Geert Uytterhoeven wrote:
> On Wed, Jun 26, 2013 at 8:54 AM, Richard Weinberger <ri...@no...> wrote:
>>> >> -#ifndef CONFIG_MMU
>>> >> +#if !CONFIG_MMU || CONFIG_UML
> FWIW, the above syntax is not correct, it should be
> 
> #if !defined(CONFIG_MMU) || defined(CONFIG_UML)
> 

Oh, really it is.  :-)


Thanks.
-- 
Chen Gang

Asianux Corporation

Re: [uml-user] [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

[Chen G. <gan...@as...>]

On 06/26/2013 02:54 PM, Richard Weinberger wrote:
> Hi!
> 
> Am 26.06.2013 08:31, schrieb Chen Gang:
>> For "User Mode Linux", it may enable 'MMU', but not need implement
>> ioremap and iounmap, so "include/asm-generic/io.h" need notice this
>> case to keep itself 'generic'.
>>
>> The related error (with allmodconfig, without pcap):
>>
>>     CC [M]  drivers/ptp/ptp_pch.o
>>   drivers/ptp/ptp_pch.c: In function �pch_remove�:
>>   drivers/ptp/ptp_pch.c:571:3: error: implicit declaration of function �iounmap� [-Werror=implicit-function-declaration]
>>   drivers/ptp/ptp_pch.c: In function �pch_probe�:
>>   drivers/ptp/ptp_pch.c:621:2: error: implicit declaration of function �ioremap� [-Werror=implicit-function-declaration]
>>   drivers/ptp/ptp_pch.c:621:13: warning: assignment makes pointer from integer without a cast [enabled by default]
>>   cc1: some warnings being treated as errors
>>
>>
>> Signed-off-by: Chen Gang <gan...@as...>
>> ---
>>  arch/um/include/asm/Kbuild |    1 +
>>  include/asm-generic/io.h   |    6 +++---
>>  2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/um/include/asm/Kbuild b/arch/um/include/asm/Kbuild
>> index b30f34a..a34ea5d 100644
>> --- a/arch/um/include/asm/Kbuild
>> +++ b/arch/um/include/asm/Kbuild
>> @@ -3,3 +3,4 @@ generic-y += hw_irq.h irq_regs.h kdebug.h percpu.h sections.h topology.h xor.h
>>  generic-y += ftrace.h pci.h io.h param.h delay.h mutex.h current.h exec.h
>>  generic-y += switch_to.h clkdev.h
>>  generic-y += trace_clock.h
>> +generic-y += io.h
> 
> We include that file already. See three lines above.
> 

Oh, really it is, thanks.

>> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
>> index d5afe96..e80331d 100644
>> --- a/include/asm-generic/io.h
>> +++ b/include/asm-generic/io.h
>> @@ -303,10 +303,10 @@ static inline void *phys_to_virt(unsigned long address)
>>  /*
>>   * Change "struct page" to physical address.
>>   *
>> - * This implementation is for the no-MMU case only... if you have an MMU
>> + * This implementation is for the no-MMU or UML case only... if you have an MMU
>>   * you'll need to provide your own definitions.
>>   */
>> -#ifndef CONFIG_MMU
>> +#if !CONFIG_MMU || CONFIG_UML
>>  static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
>>  {
>>  	return (void __iomem*) (unsigned long)offset;
>> @@ -325,7 +325,7 @@ static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
>>  static inline void iounmap(void __iomem *addr)
>>  {
>>  }
>> -#endif /* CONFIG_MMU */
>> +#endif /* !CONFIG_MMU || CONFIG_UML */
>>  
>>  #ifdef CONFIG_HAS_IOPORT
>>  #ifndef CONFIG_GENERIC_IOMAP
>>
> 
> UML has no io memory but a MMU, so I'd argue that you better fix drivers/ptp/ptp_pch.c dependencies.
> _If_ ptp_pch.c really works without real io memory, you can look what I did in my GENERIC_IO series[1]
> to make nandsim work on UML. Maybe this helps.
> 

But "no io memory" is not the excuse to not define the related dummy
function.

The drivers internal code has already check the related return value,
so it is the architecture's duty to 'tell' the driver whether support
io memory (e.g. define ioremap, but return NULL).

So all together, I recommend the fix like below

--------------------------diff begin------------------------------------

diff --git a/arch/um/include/asm/Kbuild b/arch/um/include/asm/Kbuild
index b30f34a..b282042 100644
--- a/arch/um/include/asm/Kbuild
+++ b/arch/um/include/asm/Kbuild
@@ -1,5 +1,5 @@
 generic-y += bug.h cputime.h device.h emergency-restart.h futex.h hardirq.h
 generic-y += hw_irq.h irq_regs.h kdebug.h percpu.h sections.h topology.h xor.h
-generic-y += ftrace.h pci.h io.h param.h delay.h mutex.h current.h exec.h
+generic-y += ftrace.h pci.h param.h delay.h mutex.h current.h exec.h
 generic-y += switch_to.h clkdev.h
 generic-y += trace_clock.h
diff --git a/arch/um/include/asm/io.h b/arch/um/include/asm/io.h
new file mode 100644
index 0000000..00f3cd8
--- /dev/null
+++ b/arch/um/include/asm/io.h
@@ -0,0 +1,21 @@
+#ifndef UML_IO_H
+#define UML_IO_H
+
+#include <asm-generic/io.h>
+
+/*
+ * UML does not support io memory, so return NULL.
+ */
+static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
+{
+	return NULL;
+}
+
+#define ioremap_nocache ioremap
+#define ioremap_wc ioremap_nocache
+
+static inline void iounmap(void __iomem *addr)
+{
+}
+
+#endif /* UML_IO_H */

--------------------------diff end------------------------------------


Thanks.
-- 
Chen Gang

Asianux Corporation

Re: [uml-user] [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

[Geert U. <ge...@li...>]

On Wed, Jun 26, 2013 at 8:54 AM, Richard Weinberger <ri...@no...> wrote:
>> -#ifndef CONFIG_MMU
>> +#if !CONFIG_MMU || CONFIG_UML

FWIW, the above syntax is not correct, it should be

#if !defined(CONFIG_MMU) || defined(CONFIG_UML)

> UML has no io memory but a MMU, so I'd argue that you better fix drivers/ptp/ptp_pch.c dependencies.
> _If_ ptp_pch.c really works without real io memory, you can look what I did in my GENERIC_IO series[1]
> to make nandsim work on UML. Maybe this helps.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@li...

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [uml-user] [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

[Richard W. <ri...@no...>]

Hi!

Am 26.06.2013 08:31, schrieb Chen Gang:
> For "User Mode Linux", it may enable 'MMU', but not need implement
> ioremap and iounmap, so "include/asm-generic/io.h" need notice this
> case to keep itself 'generic'.
> 
> The related error (with allmodconfig, without pcap):
> 
>     CC [M]  drivers/ptp/ptp_pch.o
>   drivers/ptp/ptp_pch.c: In function ‘pch_remove’:
>   drivers/ptp/ptp_pch.c:571:3: error: implicit declaration of function ‘iounmap’ [-Werror=implicit-function-declaration]
>   drivers/ptp/ptp_pch.c: In function ‘pch_probe’:
>   drivers/ptp/ptp_pch.c:621:2: error: implicit declaration of function ‘ioremap’ [-Werror=implicit-function-declaration]
>   drivers/ptp/ptp_pch.c:621:13: warning: assignment makes pointer from integer without a cast [enabled by default]
>   cc1: some warnings being treated as errors
> 
> 
> Signed-off-by: Chen Gang <gan...@as...>
> ---
>  arch/um/include/asm/Kbuild |    1 +
>  include/asm-generic/io.h   |    6 +++---
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/um/include/asm/Kbuild b/arch/um/include/asm/Kbuild
> index b30f34a..a34ea5d 100644
> --- a/arch/um/include/asm/Kbuild
> +++ b/arch/um/include/asm/Kbuild
> @@ -3,3 +3,4 @@ generic-y += hw_irq.h irq_regs.h kdebug.h percpu.h sections.h topology.h xor.h
>  generic-y += ftrace.h pci.h io.h param.h delay.h mutex.h current.h exec.h
>  generic-y += switch_to.h clkdev.h
>  generic-y += trace_clock.h
> +generic-y += io.h

We include that file already. See three lines above.

> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> index d5afe96..e80331d 100644
> --- a/include/asm-generic/io.h
> +++ b/include/asm-generic/io.h
> @@ -303,10 +303,10 @@ static inline void *phys_to_virt(unsigned long address)
>  /*
>   * Change "struct page" to physical address.
>   *
> - * This implementation is for the no-MMU case only... if you have an MMU
> + * This implementation is for the no-MMU or UML case only... if you have an MMU
>   * you'll need to provide your own definitions.
>   */
> -#ifndef CONFIG_MMU
> +#if !CONFIG_MMU || CONFIG_UML
>  static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
>  {
>  	return (void __iomem*) (unsigned long)offset;
> @@ -325,7 +325,7 @@ static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
>  static inline void iounmap(void __iomem *addr)
>  {
>  }
> -#endif /* CONFIG_MMU */
> +#endif /* !CONFIG_MMU || CONFIG_UML */
>  
>  #ifdef CONFIG_HAS_IOPORT
>  #ifndef CONFIG_GENERIC_IOMAP
> 

UML has no io memory but a MMU, so I'd argue that you better fix drivers/ptp/ptp_pch.c dependencies.
_If_ ptp_pch.c really works without real io memory, you can look what I did in my GENERIC_IO series[1]
to make nandsim work on UML. Maybe this helps.

Thanks,
//richard

[1] http://lists.infradead.org/pipermail/linux-mtd/2012-February/039701.html

[uml-user] [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

[Chen G. <gan...@as...>]

For "User Mode Linux", it may enable 'MMU', but not need implement
ioremap and iounmap, so "include/asm-generic/io.h" need notice this
case to keep itself 'generic'.

The related error (with allmodconfig, without pcap):

    CC [M]  drivers/ptp/ptp_pch.o
  drivers/ptp/ptp_pch.c: In function ‘pch_remove’:
  drivers/ptp/ptp_pch.c:571:3: error: implicit declaration of function ‘iounmap’ [-Werror=implicit-function-declaration]
  drivers/ptp/ptp_pch.c: In function ‘pch_probe’:
  drivers/ptp/ptp_pch.c:621:2: error: implicit declaration of function ‘ioremap’ [-Werror=implicit-function-declaration]
  drivers/ptp/ptp_pch.c:621:13: warning: assignment makes pointer from integer without a cast [enabled by default]
  cc1: some warnings being treated as errors


Signed-off-by: Chen Gang <gan...@as...>
---
 arch/um/include/asm/Kbuild |    1 +
 include/asm-generic/io.h   |    6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/um/include/asm/Kbuild b/arch/um/include/asm/Kbuild
index b30f34a..a34ea5d 100644
--- a/arch/um/include/asm/Kbuild
+++ b/arch/um/include/asm/Kbuild
@@ -3,3 +3,4 @@ generic-y += hw_irq.h irq_regs.h kdebug.h percpu.h sections.h topology.h xor.h
 generic-y += ftrace.h pci.h io.h param.h delay.h mutex.h current.h exec.h
 generic-y += switch_to.h clkdev.h
 generic-y += trace_clock.h
+generic-y += io.h
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index d5afe96..e80331d 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -303,10 +303,10 @@ static inline void *phys_to_virt(unsigned long address)
 /*
  * Change "struct page" to physical address.
  *
- * This implementation is for the no-MMU case only... if you have an MMU
+ * This implementation is for the no-MMU or UML case only... if you have an MMU
  * you'll need to provide your own definitions.
  */
-#ifndef CONFIG_MMU
+#if !CONFIG_MMU || CONFIG_UML
 static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 {
 	return (void __iomem*) (unsigned long)offset;
@@ -325,7 +325,7 @@ static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 static inline void iounmap(void __iomem *addr)
 {
 }
-#endif /* CONFIG_MMU */
+#endif /* !CONFIG_MMU || CONFIG_UML */
 
 #ifdef CONFIG_HAS_IOPORT
 #ifndef CONFIG_GENERIC_IOMAP
-- 
1.7.7.6

Re: [uml-user] Configure UML to work in IPv6 mode

[Michael R. <mc...@sa...>]

madhusudan r <mad...@ya...> wrote:

    > You're right about 'ifconfig' belonging to busybox. But things work fine for
    > IPv4 with exactly the same procedure.

Right... so busybox ifconfig doesn't speak enough IPv6, perhaps.

    > Also, in IPv6, eth1 comes up with link-local address; it just doesn't get
    > assigned the specified address. Assigning the v6 address from the command-line
    > works fine though.

Please say this more clearly.
You said that ifconfig failed.
Are you now saying that ifconfig works?
Or did you do ip?

--
]               Never tell me the odds!                 | ipv6 mesh networks [
]   Michael Richardson, Sandelman Software Works        | network architect  [
]     mc...@sa...  http://www.sandelman.ca/        |   ruby on rails    [

Re: [uml-user] Configure UML to work in IPv6 mode

[madhusudan r <mad...@ya...>]

You're right about 'ifconfig' belonging to busybox. But things work fine for IPv4 with exactly the same procedure.

Also, in IPv6, eth1 comes up with link-local address; it just doesn't get assigned the specified address. Assigning the v6 address from the command-line works fine though.



________________________________
 From: Michael Richardson <mc...@sa...>
To: madhusudan r <mad...@ya...> 
Cc: "use...@li..." <use...@li...> 
Sent: Monday, 24 June 2013 8:17 PM
Subject: Re: [uml-user] Configure UML to work in IPv6 mode
 


Since you seem to have IPv6 module loaded, and IPv6 addresses in your
ifconfig output, but your ifconfig won't add it.
If your 'ifconfig' from busybox?  Maybe it is too limited to do the work.
You mention WRT, which is why I suspect ifconfig is really at fault.

But, your ifconfig line seems wrong to me:
     ifconfig eth1 inet6 2001:220:abcd:70::10/64 up

would seem to be the right incantation.

I usually do:
  ip -6 add addr 2001:220:abcd:70::10/64 dev eth1

--
]               Never tell me the odds!                 | ipv6 mesh networks [
]   Michael Richardson, Sandelman Software Works        | network architect  [
]    mc...@sa...  http://www.sandelman.ca/        |   ruby on rails    [

Re: [uml-user] Configure UML to work in IPv6 mode

[Michael R. <mc...@sa...>]

Since you seem to have IPv6 module loaded, and IPv6 addresses in your
ifconfig output, but your ifconfig won't add it.
If your 'ifconfig' from busybox?  Maybe it is too limited to do the work.
You mention WRT, which is why I suspect ifconfig is really at fault.

But, your ifconfig line seems wrong to me:
     ifconfig eth1 inet6 2001:220:abcd:70::10/64 up

would seem to be the right incantation.

I usually do:
  ip -6 add addr 2001:220:abcd:70::10/64 dev eth1

--
]               Never tell me the odds!                 | ipv6 mesh networks [
]   Michael Richardson, Sandelman Software Works        | network architect  [
]     mc...@sa...  http://www.sandelman.ca/        |   ruby on rails    [

Re: [uml-user] Configure UML to work in IPv6 mode

[madhusudan r <mad...@ya...>]

root@OpenWrt:/# uname -a
Linux OpenWrt 3.2.15-00054-g0fccead-dirty #6 Thu Jun 20 22:19:27 IST 2013 x86_64 GNU/Linux

I have a configured a bridge and a tap interface on the host system. The tap interface is bridged with eth1 of the UML.

user@ubuntu-10784976:/etc/init.d$ ifconfig br1
br1       Link encap:Ethernet  HWaddr 1a:65:a0:13:71:84
          inet6 addr: 2001:220:abcd:70::1/64 Scope:Global
          BROADCAST MULTICAST  MTU:1500  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0
          RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)

user@ubuntu-10784976:/etc/init.d$ ifconfig tap2
tap2      Link encap:Ethernet  HWaddr 1a:65:a0:13:71:84
          inet6 addr: fe80::1865:a0ff:fe13:7184/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:90644 errors:0 dropped:0 overruns:0 frame:0
          TX packets:116 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:500
          RX bytes:36438888 (36.4 MB)  TX bytes:13932 (13.9 KB)

user@ubuntu-10784976:/etc/init.d$ sudo brctl show
bridge name     bridge id               STP enabled     interfaces
br0             8000.6af7bb7f21a5       no              tap0
                                                        tap1
br1             8000.1a65a0137184       no              tap2



________________________________
 From: madhusudan r <mad...@ya...>
To: "use...@li..." <use...@li...> 
Sent: Monday, 24 June 2013 10:03 AM
Subject: [uml-user] Configure UML to work in IPv6 mode
 


I'm trying to run an UML in IPv6 mode on a Linux (Ubuntu) box, and have not been successful with it. I could get the UML to boot up alright, but the interfaces do not 
seem to accept IPv6 addresses. The file /etc/config/network looks like 
this:

config interface lan
        option ifname   'eth1'
        option proto    'static'
        option ip6addr  '2001:220:abcd:70::10/64'

Adding it manually doesn’t seem to help either:

root@OpenWrt:/# ifconfig eth1 add 2001:220:abcd:70::10/64
ifconfig: socket(AF_INET6,2,0): Address family not supported by protocol
root@OpenWrt:/# ip -f inet6 addr add dev eth1 2001:220:abcd:70::10/64
RTNETLINK answers: Operation not supported
root@OpenWrt:/# ip -f inet6 addr add dev eth1 2001:220:abcd:70::10
RTNETLINK answers: Operation not supported
root@OpenWrt:/# ip addr add dev eth1 2001:220:abcd:70::10/64
RTNETLINK
 answers: Operation not supported

I referenced a few OpenWRT links that talk about how to enable IPv6 on 
the system, and believe I have it in place. Could you please help me 
understand what is missing or what is the problem? 


I'm not sure what relevant information to provide. Please let me know.

This is what is displayed as the startup banner:

BusyBox v1.19.4 (2013-02-10 13:17:40 GMT) built-in shell (ash)
Enter 'help' for a list of built-in commands.
  _______                     ________        __
|       |.-----.-----.-----.|  |  |  |.----.|  |_
|   -   ||  _  |  -__|     ||  |  |  ||   _||   _|
|_______||   __|_____|__|__||________||__|  |____|
          |__| W I R E L E S S   F R E E D O M
-----------------------------------------------------
ATTITUDE ADJUSTMENT (Bleeding Edge, r33315)
-----------------------------------------------------
  * 1/4 oz Vodka      Pour all ingredients into
 mixing
  * 1/4 oz Gin        tin with ice, strain into glass.
  * 1/4 oz Amaretto
  * 1/4 oz Triple sec
  * 1/4 oz Peach schnapps
  * 1/4 oz Sour mix
  * 1 splash Cranberry juice
-----------------------------------------------------
root@OpenWrt:/#
root@(none):/# opkg install kmod-ipv6
Package kmod-ipv6 (3.2.15-1) installed in root is up to date.
root@OpenWrt:/#

Thanks!
------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
_______________________________________________
User-mode-linux-user mailing list
Use...@li...
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-user

[uml-user] Configure UML to work in IPv6 mode

[madhusudan r <mad...@ya...>]

I'm trying to run an UML in IPv6 mode on a Linux (Ubuntu) box, and have not been successful with it. I could get the UML to boot up alright, but the interfaces do not 
seem to accept IPv6 addresses. The file /etc/config/network looks like 
this:

config interface lan
        option ifname   'eth1'
        option proto    'static'
        option ip6addr  '2001:220:abcd:70::10/64'

Adding it manually doesn’t seem to help either:

root@OpenWrt:/# ifconfig eth1 add 2001:220:abcd:70::10/64
ifconfig: socket(AF_INET6,2,0): Address family not supported by protocol
root@OpenWrt:/# ip -f inet6 addr add dev eth1 2001:220:abcd:70::10/64
RTNETLINK answers: Operation not supported
root@OpenWrt:/# ip -f inet6 addr add dev eth1 2001:220:abcd:70::10
RTNETLINK answers: Operation not supported
root@OpenWrt:/# ip addr add dev eth1 2001:220:abcd:70::10/64
RTNETLINK answers: Operation not supported

I referenced a few OpenWRT links that talk about how to enable IPv6 on 
the system, and believe I have it in place. Could you please help me 
understand what is missing or what is the problem? 


I'm not sure what relevant information to provide. Please let me know.

This is what is displayed as the startup banner:

BusyBox v1.19.4 (2013-02-10 13:17:40 GMT) built-in shell (ash)
Enter 'help' for a list of built-in commands.
  _______                     ________        __
|       |.-----.-----.-----.|  |  |  |.----.|  |_
|   -   ||  _  |  -__|     ||  |  |  ||   _||   _|
|_______||   __|_____|__|__||________||__|  |____|
          |__| W I R E L E S S   F R E E D O M
-----------------------------------------------------
ATTITUDE ADJUSTMENT (Bleeding Edge, r33315)
-----------------------------------------------------
  * 1/4 oz Vodka      Pour all ingredients into mixing
  * 1/4 oz Gin        tin with ice, strain into glass.
  * 1/4 oz Amaretto
  * 1/4 oz Triple sec
  * 1/4 oz Peach schnapps
  * 1/4 oz Sour mix
  * 1 splash Cranberry juice
-----------------------------------------------------
root@OpenWrt:/#
root@(none):/# opkg install kmod-ipv6
Package kmod-ipv6 (3.2.15-1) installed in root is up to date.
root@OpenWrt:/#

Thanks!

Re: [uml-user] [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.

[Arnd B. <ar...@ar...>]

On Tuesday 28 May 2013, H. Peter Anvin wrote:
> On 05/28/2013 08:43 AM, Arnd Bergmann wrote:
> > 
> > Right, that is what the patch I just posted does.
> > 
> > On a related note, I found that WARN_ON() can no longer be compiled
> > out since there is already code that relies on the side-effects of
> > the condition. I assume that was an intentional change I missed,
> > since it used to be defined so that you could remove it completely.
> > 
> 
> It is possible to define WARN_ON() as:
> 
> #define WARN_ON(x) ((void)(x))
> 
> ... which preserves side effects.

Yes, actually the return value has to be maintained as well.
The current (!CONFIG_BUG) default implementation is

#define WARN_ON(condition) ({                                           \
        int __ret_warn_on = !!(condition);                              \
        unlikely(__ret_warn_on);                                        \
})

which seems fine.

#define WARN_ON(condition) unlikely(!!(condition))

is probably just as good.

	Arnd

Re: [uml-user] [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.

[H. P. A. <hp...@zy...>]

On 05/28/2013 08:43 AM, Arnd Bergmann wrote:
> 
> Right, that is what the patch I just posted does.
> 
> On a related note, I found that WARN_ON() can no longer be compiled
> out since there is already code that relies on the side-effects of
> the condition. I assume that was an intentional change I missed,
> since it used to be defined so that you could remove it completely.
> 

It is possible to define WARN_ON() as:

#define WARN_ON(x) ((void)(x))

... which preserves side effects.

	-hpa

Re: [uml-user] [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.

[Arnd B. <ar...@ar...>]

On Tuesday 28 May 2013, H. Peter Anvin wrote:
> On 05/28/2013 01:19 AM, Ingo Molnar wrote:
> > 
> > So I think the same principle applies to it as to any other debugging 
> > code: it's fine to be able to turn debugging off. It's a performance 
> > versus kernel robustness/determinism trade-off.
> > 
> 
> I suspect, rather, that BUG() should turn into a trap (or jump to a
> death routine) under any circumstances.  The one thing that can be
> omitted for small configurations are the annotations, which only serve
> to output a more human-readable error message.

Right, that is what the patch I just posted does.

On a related note, I found that WARN_ON() can no longer be compiled
out since there is already code that relies on the side-effects of
the condition. I assume that was an intentional change I missed,
since it used to be defined so that you could remove it completely.

	Arnd

Re: [uml-user] [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.

[H. P. A. <hp...@zy...>]

On 05/28/2013 01:19 AM, Ingo Molnar wrote:
> 
> So I think the same principle applies to it as to any other debugging 
> code: it's fine to be able to turn debugging off. It's a performance 
> versus kernel robustness/determinism trade-off.
> 

I suspect, rather, that BUG() should turn into a trap (or jump to a
death routine) under any circumstances.  The one thing that can be
omitted for small configurations are the annotations, which only serve
to output a more human-readable error message.

	-hpa

Re: [uml-user] [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.

[Arnd B. <ar...@ar...>]

On Tuesday 28 May 2013 10:19:10 Ingo Molnar wrote:
> 
> * Russell King - ARM Linux <li...@ar...> wrote:
> 
> > So, if you want to use this, then you should update the CONFIG_BUG text 
> > to include a warning to this effect:
> > 
> >      Warning: if CONFIG_BUG is turned off, and control flow reaches
> >      a BUG(), the system behaviour will be undefined.
> > 
> > so that people can make an informed choice about this, because at the
> > moment:
> > 
> >           Disabling this option eliminates support for BUG and WARN, reducing
> >           the size of your kernel image and potentially quietly ignoring
> >           numerous fatal conditions. You should only consider disabling this
> >           option for embedded systems with no facilities for reporting errors.
> >           Just say Y.
> > 
> > will become completely misleading.  Turning this option off will _not_ 
> > result in "quietly ignoring numerous fatal conditions".
> 
> I'm fine with adding your text as a clarification - but I think 'quietly 
> ignoring fatal conditions' very much implies an undefined outcome if that 
> unexpected condition does occur: the code might crash, it might corrupt 
> memory or it might do some other unexpected thing.
> 
> There are many other places that do a BUG_ON() of a NULL pointer or so, or 
> of a zero refcount, or a not held lock - and turning the BUG_ON() off 
> makes the code unpredictable _anyway_ - even if the compiler does not 
> notice an uninitialized variable.
> 
> So pretty much any weakening of BUG_ON() _will_ make the kernel more 
> unpredictable.

FWIW, I've run some size analyis using the ARM 'multi_v7_defconfig'
and gcc-4.8, using various definitions for BUG() and BUG_ON(), to
see how big the size improvement actually gets

1. Baseline: normal bug plus CONFIG_BUG_VERBOSE
   text    data     bss     dec     hex filename
3743196  224396  206812 4174404  3fb244 vmlinux-bugverbose

2. disabling CONFIG_BUG_VERBOSE, saving 0.6%
3716920  224380  206812 4148112  3f4b90 vmlinux-nobugverbose

3. #define BUG() panic(__func__), #define BUG_ON(c) if(c) BUG(), saving 1.0%
3701076  224384  206812 4132272  3f0db0 vmlinux-bug-panicfunc

3. #define BUG() panic(__func__), #define BUG_ON(c) if(c) BUG(), saving 1.5%
3678884  224400  206812 4110096  3eb710 vmlinux-bug-panic

4. #define BUG() unreachable(), saving 2.1%
3652636  224384  206812 4083832  3e5078 vmlinux-bug-unreachable

5. as 4, plus #define BUG_ON(c) if(c) unreachable(), saving 2.2%
3651108  224380  206812 4082300  3e4a7c vmlinux-bugon-unreachable

6. #define BUG() do{}while(0), saving 2.1%
3654264  224380  206812 4085456  3e56d0 vmlinux-nobug

7. as 6, plus #define BUG_ON if(0 && (c)) unreachable, saving 2.2%
3648392  223996  206748 4079136  3e3e20 vmlinux-no-bugon

8. my patch below, saving 1.8%
3666532  224380  206812 4097724  3e86bc obj-tmp/vmlinux

The gain of doing unreachable and letting the code run off whereever
is minimal compared to the current state of doing nothing, but it
avoids the warnings.

Same test using x86_defconfig:

1. CONFIG_BUG=y, CONFIG_BUGVERBOSE=n
10797859        1395648 1175552 13369059         cbfee3 vmlinux-x86-bug

2. CONFIG_BUG=n, saves 1.0%
10658553        1395584 1175552 13229689         c9de79 vmlinux-x86-nobug

3. with my patch, saves 0.8%
10684186        1395584 1175552 13255322         ca429a vmlinux-x86-archbug

Getting 1-2% savings in kernel size seems absolutely worth keeping the
option, but 0.2-0.4% left for getting reproducible behavior also seems
worthwhile. The result in the patch below.

This basically loses any of the BUG() reporting, but leaves the
logic to trap and kill the task in place when CONFIG_BUG is disabled.

Signed-off-by: Arnd Bergmann <ar...@ar...>

diff --git a/arch/arm/include/asm/bug.h b/arch/arm/include/asm/bug.h
index 7af5c6c..f25e638 100644
--- a/arch/arm/include/asm/bug.h
+++ b/arch/arm/include/asm/bug.h
@@ -3,8 +3,6 @@
 
 #include <linux/linkage.h>
 
-#ifdef CONFIG_BUG
-
 /*
  * Use a suitable undefined instruction to use for ARM/Thumb2 bug handling.
  * We need to be careful not to conflict with those used by other modules and
@@ -51,10 +50,10 @@ do {								\
 	asm volatile(BUG_INSTR_TYPE #__value);			\
 	unreachable();						\
 } while (0)
 #endif  /* CONFIG_DEBUG_BUGVERBOSE */
 
 #define HAVE_ARCH_BUG
-#endif  /* CONFIG_BUG */
+#define HAVE_ARCH_BUG_ON
 
 #include <asm-generic/bug.h>
 
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index 2f03ff0..ba38ebb 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -1,7 +1,6 @@
 #ifndef _ASM_X86_BUG_H
 #define _ASM_X86_BUG_H
 
-#ifdef CONFIG_BUG
 #define HAVE_ARCH_BUG
 
 #ifdef CONFIG_DEBUG_BUGVERBOSE
@@ -33,8 +32,6 @@ do {								\
 } while (0)
 #endif
 
-#endif /* !CONFIG_BUG */
-
 #include <asm-generic/bug.h>
 
 #endif /* _ASM_X86_BUG_H */
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 7d10f96..bbd2872 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -112,12 +112,13 @@ extern void warn_slowpath_null(const char *file, const int line);
 #endif
 
 #ifndef HAVE_ARCH_BUG_ON
-#define BUG_ON(condition) do { if (condition) ; } while(0)
+#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while(0)
 #endif
 
 #ifndef HAVE_ARCH_WARN_ON
 #define WARN_ON(condition) ({						\
 	int __ret_warn_on = !!(condition);				\
+	(void)__ret_warn_on;						\
 	unlikely(__ret_warn_on);					\
 })
 #endif
@@ -125,6 +126,8 @@ extern void warn_slowpath_null(const char *file, const int line);
 #ifndef WARN
 #define WARN(condition, format...) ({					\
 	int __ret_warn_on = !!(condition);				\
+	if (0 && (__ret_warn_on))					\
+		printk(format);						\
 	unlikely(__ret_warn_on);					\
 })
 #endif

Re: [uml-user] [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.

[Chen G. <gan...@as...>]

On 05/28/2013 04:19 PM, Ingo Molnar wrote:
>> > And I come back to one of my previous arguments - is it not better to 
>> > panic() if we hit one of these conditions so that the system can try to 
>> > do a panic-reboot rather than continue blindly into the unknown?
> It will often continue blindly into the unknown even if the compiler is 
> happy ...
> 

For Server, it is necessary to always enable CONFIG_BUG and call panic()

When analyzing core dump or KDB trap, we have to assume that the kernel
has already "continued blindly", but lucky, we can get the core dump or
KDB trap finally (sometimes, we really even can not get core dump or KDB
trap).


For PC, it is useless to disable CONFIG_BUG

The PC memory has already large enough to skip the minimal size
optimization. And its speed is also high enough to skip the speed
improvement by minimal size optimization.


For Embedded system, some of architectures may disable CONFIG_BUG.

Embedded system are widely used in many area, so the requirement of each
architectures for BUG() may be different,

  some may need reboot as quickly as possible for urgent processing;
  some may need dead looping in BUG() for avoid user amazing;
    (if auto-reboot, users feel out of control, don't know what happens)
  some may still need panic() just like Server requirements.
  others may not care about it, just implement it as minimal size.



> The only difference is that i

t's "unpredictable" in a way not visible from
> the C code: the code won't necessarily fall through the BUG() when hitting 
> that condition - although in practice it probably will.
> 
> So I think the same principle applies to it as to any other debugging 
> code: it's fine to be able to turn debugging off. It's a performance 
> versus kernel robustness/determinism trade-off.

'minimal size' for BUG() is belongs to some of Embedded system specific
features, it is not 'generic' enough to be in "include/asm-generic/".

If we still provide the "disable CONFIG_BUG", some of 'crazy users'
(e.g. randconfig) may make 'noise' to most of architectures.

So we need not provide "disable CONFIG_BUG" features for all platforms,
since most of architectures need not support it, and the architecture
which really need minimal size, can implement it by itself as a
architecture specific feature.



Thanks.
-- 
Chen Gang

Asianux Corporation