Re: [PATCH 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid

[christophe leroy]

Le 02/03/2018 à 20:54, Mathieu Malaterre a écrit :

On Mon, Feb 26, 2018 at 9:46 AM, Segher Boessenkool
 wrote:

On Mon, Feb 26, 2018 at 07:32:03AM +0100, Christophe LEROY wrote:

Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :

-#define pfn_valid(pfn)  ((pfn) >= ARCH_PFN_OFFSET && (pfn) <
max_mapnr)
+#define pfn_valid(pfn) \
+(((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))


What will happen when ARCH_PFN_OFFSET is not nul and pfn is lower than
ARCH_PFN_OFFSET ?


It will work fine.

Say you are asking for  a <= x < b  so (in actual integers, no overflow)
that is  0 <= x-a < b-a  and you also assume x-a overflows, so that we
are actually comparing  x-a+M < b-a  with M = 2**32 or such (the maximum
value in the unsigned integer type plus one).  This comparison is
obviously always false.

(It also works if b < a btw).



Thanks Segher !

Christophe does that clarify things or do you want me to update the
commit message ?



No it is fine for me.

Reviewed-by: Christophe Leroy 

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel 
antivirus Avast.
https://www.avast.com/antivirus

Re: [PATCH v2 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok

[christophe leroy]

Le 02/03/2018 à 20:50, Mathieu Malaterre a écrit :

Rewrite function-like macro into regular static inline function to avoid a
warning during macro expansion.
Fix warning (treated as error in W=1):

   CC  arch/powerpc/kernel/signal_32.o
In file included from ./include/linux/uaccess.h:14:0,
  from ./include/asm-generic/termios-base.h:8,
  from ./arch/powerpc/include/asm/termios.h:20,
  from ./include/uapi/linux/termios.h:6,
  from ./include/linux/tty.h:7,
  from arch/powerpc/kernel/signal_32.c:36:
./include/asm-generic/termios-base.h: In function 
‘user_termio_to_kernel_termios’:
./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned 
expression >= 0 is always true [-Werror=type-limits]
(((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)
^
./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro 
‘__access_ok’
__access_ok((__force unsigned long)(addr), (size), get_fs()))
^~~
./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro 
‘access_ok’
   if (access_ok(VERIFY_READ, __gu_addr, (size)))   \
   ^
./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro 
‘__get_user_check’
   __get_user_check((x), (ptr), sizeof(*(ptr)))
   ^~~~
./include/asm-generic/termios-base.h:36:6: note: in expansion of macro 
‘get_user’
   if (get_user(termios->c_line, >c_line) < 0)
   ^~~~
[...]
cc1: all warnings being treated as errors

Suggested-by: Segher Boessenkool 
Signed-off-by: Mathieu Malaterre 


Reviewed-by: Christophe Leroy 


---
  arch/powerpc/include/asm/uaccess.h | 10 +++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/uaccess.h 
b/arch/powerpc/include/asm/uaccess.h
index 51bfeb8777f0..a62ee663b2c8 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -47,9 +47,13 @@
  
  #else
  
-#define __access_ok(addr, size, segment)	\

-   (((addr) <= (segment).seg) &&\
-(((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)
+static inline int __access_ok(unsigned long addr, unsigned long size,
+   mm_segment_t seg)
+{
+   if (addr > seg.seg)
+   return 0;
+   return (size == 0 || size - 1 <= seg.seg - addr);
+}
  
  #endif
  



---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel 
antivirus Avast.
https://www.avast.com/antivirus

Re: [PATCH v3 02/10] include: Move compat_timespec/ timeval to compat_time.h

[James Hogan]

On Mon, Jan 15, 2018 at 06:18:10PM -0800, Deepa Dinamani wrote:
> All the current architecture specific defines for these
> are the same. Refactor these common defines to a common
> header file.
> 
> The new common linux/compat_time.h is also useful as it
> will eventually be used to hold all the defines that
> are needed for compat time types that support non y2038
> safe types. New architectures need not have to define these
> new types as they will only use new y2038 safe syscalls.
> This file can be deleted after y2038 when we stop supporting
> non y2038 safe syscalls.

...

>  arch/mips/include/asm/compat.h| 11 ---
>  arch/mips/kernel/signal32.c   |  2 +-

For MIPS:
Acked-by: James Hogan 

Cheers
James


signature.asc
Description: Digital signature

Re: [PATCH 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid

[Mathieu Malaterre]

On Mon, Feb 26, 2018 at 9:46 AM, Segher Boessenkool
 wrote:
> On Mon, Feb 26, 2018 at 07:32:03AM +0100, Christophe LEROY wrote:
>> Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
>> >-#define pfn_valid(pfn)  ((pfn) >= ARCH_PFN_OFFSET && (pfn) <
>> >max_mapnr)
>> >+#define pfn_valid(pfn) \
>> >+(((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))
>>
>> What will happen when ARCH_PFN_OFFSET is not nul and pfn is lower than
>> ARCH_PFN_OFFSET ?
>
> It will work fine.
>
> Say you are asking for  a <= x < b  so (in actual integers, no overflow)
> that is  0 <= x-a < b-a  and you also assume x-a overflows, so that we
> are actually comparing  x-a+M < b-a  with M = 2**32 or such (the maximum
> value in the unsigned integer type plus one).  This comparison is
> obviously always false.
>
> (It also works if b < a btw).
>
>
Thanks Segher !

Christophe does that clarify things or do you want me to update the
commit message ?

[PATCH v2 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok

[Mathieu Malaterre]

Rewrite function-like macro into regular static inline function to avoid a
warning during macro expansion.
Fix warning (treated as error in W=1):

  CC  arch/powerpc/kernel/signal_32.o
In file included from ./include/linux/uaccess.h:14:0,
 from ./include/asm-generic/termios-base.h:8,
 from ./arch/powerpc/include/asm/termios.h:20,
 from ./include/uapi/linux/termios.h:6,
 from ./include/linux/tty.h:7,
 from arch/powerpc/kernel/signal_32.c:36:
./include/asm-generic/termios-base.h: In function 
‘user_termio_to_kernel_termios’:
./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned 
expression >= 0 is always true [-Werror=type-limits]
   (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)
   ^
./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro 
‘__access_ok’
   __access_ok((__force unsigned long)(addr), (size), get_fs()))
   ^~~
./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro 
‘access_ok’
  if (access_ok(VERIFY_READ, __gu_addr, (size)))   \
  ^
./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro 
‘__get_user_check’
  __get_user_check((x), (ptr), sizeof(*(ptr)))
  ^~~~
./include/asm-generic/termios-base.h:36:6: note: in expansion of macro 
‘get_user’
  if (get_user(termios->c_line, >c_line) < 0)
  ^~~~
[...]
cc1: all warnings being treated as errors

Suggested-by: Segher Boessenkool 
Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/include/asm/uaccess.h | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/uaccess.h 
b/arch/powerpc/include/asm/uaccess.h
index 51bfeb8777f0..a62ee663b2c8 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -47,9 +47,13 @@
 
 #else
 
-#define __access_ok(addr, size, segment)   \
-   (((addr) <= (segment).seg) &&   \
-(((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)
+static inline int __access_ok(unsigned long addr, unsigned long size,
+   mm_segment_t seg)
+{
+   if (addr > seg.seg)
+   return 0;
+   return (size == 0 || size - 1 <= seg.seg - addr);
+}
 
 #endif
 
-- 
2.11.0

[PATCH v2 01/21] powerpc: Remove warning on array size when empty

[Mathieu Malaterre]

When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
array feature_properties is defined as an empty array, which in turn
triggers the following warning (treated as error on W=1):

  CC  arch/powerpc/kernel/prom.o
arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression < 0 
is always false [-Werror=type-limits]
  for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
^
cc1: all warnings being treated as errors

Suggested-by: Michael Ellerman 
Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/kernel/prom.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 4dffef947b8a..330c65f04820 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned long node)
 
 static void __init check_cpu_feature_properties(unsigned long node)
 {
-   unsigned long i;
+   int i;
struct feature_property *fp = feature_properties;
const __be32 *prop;
 
-   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
+   for (i = 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp) {
prop = of_get_flat_dt_prop(node, fp->name, NULL);
if (prop && be32_to_cpup(prop) >= fp->min_value) {
cur_cpu_spec->cpu_features |= fp->cpu_feature;
-- 
2.11.0

Re: [PATCH 1/2] crypto: talitos - don't persistently map req_ctx->hw_context and req_ctx->buf

[Horia Geantă]

On 2/26/2018 6:40 PM, Christophe Leroy wrote:
> Commit 49f9783b0cea ("crypto: talitos - do hw_context DMA mapping
> outside the requests") introduced a persistent dma mapping of
> req_ctx->hw_context
> Commit 37b5e8897eb5 ("crypto: talitos - chain in buffered data for ahash
> on SEC1") introduced a persistent dma mapping of req_ctx->buf
> 
> As there is no destructor for req_ctx (the request context), the
> associated dma handlers where set in ctx (the tfm context). This is
> wrong as several hash operations can run with the same ctx.
> 
> This patch removes this persistent mapping.
> 
> Reported-by: Horia Geanta 
> Fixes: 49f9783b0cea ("crypto: talitos - do hw_context DMA mapping outside the 
> requests")
> Fixes: 37b5e8897eb5 ("crypto: talitos - chain in buffered data for ahash on 
> SEC1")
> Signed-off-by: Christophe Leroy 
Tested-by: Horia Geantă 

Please add this to 4.15.y -stable tree.

Thanks,
Horia

Re: [PATCH 17/18] crypto: talitos - chain in buffered data for ahash on SEC1

[Christophe LEROY]

Le 02/03/2018 à 18:27, Horia Geantă a écrit :

On 10/6/2017 4:05 PM, Christophe Leroy wrote:
[...]

@@ -1778,6 +1814,36 @@ static int common_nonsnoop_hash(struct talitos_edesc 
*edesc,
if (is_sec1 && from_talitos_ptr_len(>ptr[3], true) == 0)
talitos_handle_buggy_hash(ctx, edesc, >ptr[3]);
  
+	if (is_sec1 && req_ctx->nbuf && length) {

+   struct talitos_desc *desc2 = desc + 1;
+   dma_addr_t next_desc;

[...]

+   next_desc = dma_map_single(dev, >hdr1, TALITOS_DESC_SIZE,
+  DMA_BIDIRECTIONAL);
+   desc->next_desc = cpu_to_be32(next_desc);

Where is desc->next_desc initialized for the !is_sec1 case?
Memory allocation is done using kmalloc(), and since desc->next_desc is checked
in some cases also for SEC 2.x+, it should be initialized to 0.


See 
https://elixir.bootlin.com/linux/v4.16-rc3/source/drivers/crypto/talitos.c#L1411


edesc = kmalloc(alloc_len, GFP_DMA | flags);
if (!edesc) {
dev_err(dev, "could not allocate edescriptor\n");
err = ERR_PTR(-ENOMEM);
goto error_sg;
}
memset(>desc, 0, sizeof(edesc->desc));


Christophe

Re: [PATCH 17/18] crypto: talitos - chain in buffered data for ahash on SEC1

[Horia Geantă]

On 10/6/2017 4:05 PM, Christophe Leroy wrote:
[...]
> @@ -1778,6 +1814,36 @@ static int common_nonsnoop_hash(struct talitos_edesc 
> *edesc,
>   if (is_sec1 && from_talitos_ptr_len(>ptr[3], true) == 0)
>   talitos_handle_buggy_hash(ctx, edesc, >ptr[3]);
>  
> + if (is_sec1 && req_ctx->nbuf && length) {
> + struct talitos_desc *desc2 = desc + 1;
> + dma_addr_t next_desc;
[...]
> + next_desc = dma_map_single(dev, >hdr1, TALITOS_DESC_SIZE,
> +DMA_BIDIRECTIONAL);
> + desc->next_desc = cpu_to_be32(next_desc);
Where is desc->next_desc initialized for the !is_sec1 case?
Memory allocation is done using kmalloc(), and since desc->next_desc is checked
in some cases also for SEC 2.x+, it should be initialized to 0.

Thanks,
Horia

Re: Hotplug + Reboot is crashing HPT guest with HPT resizing enabled

[Bharata B Rao]

On Fri, Mar 2, 2018 at 7:51 AM, David Gibson 
wrote:

> On Fri, Feb 23, 2018 at 03:02:40PM +0530, Bharata B Rao wrote:
> > Hi,
> >
> > Rebooting a hash guest after hotplugging memory to it is crashing the
> > guest. This is seen only when HPT resizing is enabled. I see guest
> crashing
> > at multiple places, but this location is fairly commonly seen:
> >
> > kernel BUG at mm/slub.c:3912!
> >
> > Testing with latest guest kernel and ppc-for-2.12 branch of QEMU.
>
> Ugh.  We had several bugs along these lines, but I thought I'd fixed
> them.  I wonder what this one is.
>
> > A bit of debugging shows me that when memory is added, the guest kernel
> > tries to resize HPT to a htab_shift value lesser than the value with
> which
> > the guest has booted. For eg. a 8GB guest boots with htab_shift of 26.
> When
> > 1G is hot-added,
> > arch/powerpc/mm/hash_utils_64.c:resize_hpt_for_hotplug() ends up
> assigning
> > 24 to target_hpt_shift. This looks suspicious as we are increasing the
> > memory, but kernel is asking for shrinking the HPT size.
>
> So the shrink-HPT-on-add-memory is actually expected and should be
> harmless.  It occurs because qemu estimates HPT size on the
> traditional HPT == RAM size / 64 formular, which was devised with 4k
> pages in mind.  The kernel on the other hand, knows it is using 64k
> pages and so estimates a smaller HPT size.  Hot plugging memory always
> prompts the guest to re-estimate the required HPT size, but if the
> added memory is small enough, that size can still be smaller than
> qemu's initial guess.
>

Thanks for the clarification.


>
> > HPT resizing
> > requests fail though, but next reboot crashes the guest.
>
> As noted the shrink is expected, so we need to debug the crash
> separately.  Do you have 9478956794c11239b7c1c3ef9ce95c883bb839a3 in
> your tree?
>

I do now and I no longer see the crash that I was observing last week.

Regards,
Bharata.

Re: [PATCH] powerpc: boot: add strrchr function

[Michael Ellerman]

Rob Herring  writes:

> libfdt gained a new dependency on strrchr, so copy the implementation
> from lib/string.c. Most of the string functions are in assembly, but
> stdio.c already has strnlen, so add strrchr there.
>
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Cc: Michael Ellerman 
> Signed-off-by: Rob Herring 
> ---
> Please ack. This is a dependency for dtc/libfdt sync with upstream.

Yeah seems fine. At some point we should try and duplicate less code in
boot, but now is not that time.

Acked-by: Michael Ellerman 

cheers

> diff --git a/arch/powerpc/boot/stdio.c b/arch/powerpc/boot/stdio.c
> index a701261b1781..98042eff7b26 100644
> --- a/arch/powerpc/boot/stdio.c
> +++ b/arch/powerpc/boot/stdio.c
> @@ -21,6 +21,16 @@ size_t strnlen(const char * s, size_t count)
>   return sc - s;
>  }
>  
> +char *strrchr(const char *s, int c)
> +{
> + const char *last = NULL;
> + do {
> + if (*s == (char)c)
> + last = s;
> + } while (*s++);
> + return (char *)last;
> +}
> +
>  #ifdef __powerpc64__
>  
>  # define do_div(n, base) ({  \
> -- 
> 2.14.1

Re: [PATCH] powerpc: dts: replace 'linux, stdout-path' with 'stdout-path'

[Michael Ellerman]

Rob Herring  writes:

> On Wed, Feb 28, 2018 at 7:33 PM, Michael Ellerman  wrote:
>> Rob Herring  writes:
>>
>>> 'linux,stdout-path' has been deprecated for some time in favor of
>>> 'stdout-path'. Now dtc will warn on occurrences of 'linux,stdout-path'.
>>> Search and replace all the of occurrences with 'stdout-path'.
>>
>> This patch looks OK.
>>
>> But please remember that not all device trees are generated with dtc, we
>> still have machines in the wild that have firmware which use
>> "linux,stdout-path" and may never be updated.
>
> Absolutely. The core code still supports both.

Phew ;)

It has prompted us to look at various firmware pieces and some we can
update to start using the new property or both. It's still going to be
many years before we can actually remove support for linux,stdout-path
though.

> The only scenario I could come up with is someone has an old
> bootloader that only understands linux,stdout-path and modifies it and
> they update the dtb. We may end up with both properties with
> stdout-path taking preference.

Yeah I think this is fairly safe, and in the unlikely case it does cause
a problem we can always back the change out for an individual file.

cheers

[RFC PATCH 10/10] powerpc/64s: Wire up cpu_show_spectre_v2()

[Michael Ellerman]

Add a definition for cpu_show_spectre_v2() to override the generic
version. This has several permuations, though in practice some may not
occur we cater for any combination.

The most verbose is:

  Mitigation: Indirect branch serialisation (kernel only), Indirect
  branch cache disabled, ori31 speculation barrier enabled

We don't treat the ori31 speculation barrier as a mitigation on its
own, because it has to be *used* by code in order to be a mitigation
and we don't know if userspace is doing that. So if that's all we see
we say:

  Vulnerable, ori31 speculation barrier enabled

Signed-off-by: Michael Ellerman 
---
 arch/powerpc/kernel/security.c | 33 +
 1 file changed, 33 insertions(+)

diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index 0eace3cac818..2cee3dcd231b 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -58,3 +58,36 @@ ssize_t cpu_show_spectre_v1(struct device *dev, struct 
device_attribute *attr, c
 
return sprintf(buf, "Vulnerable\n");
 }
+
+ssize_t cpu_show_spectre_v2(struct device *dev, struct device_attribute *attr, 
char *buf)
+{
+   bool bcs, ccd, ori;
+   struct seq_buf s;
+
+   seq_buf_init(, buf, PAGE_SIZE - 1);
+
+   bcs = security_ftr_enabled(SEC_FTR_BCCTRL_SERIALISED);
+   ccd = security_ftr_enabled(SEC_FTR_COUNT_CACHE_DISABLED);
+   ori = security_ftr_enabled(SEC_FTR_SPEC_BAR_ORI31);
+
+   if (bcs || ccd) {
+   seq_buf_printf(, "Mitigation: ");
+
+   if (bcs)
+   seq_buf_printf(, "Indirect branch serialisation 
(kernel only)");
+
+   if (bcs && ccd)
+   seq_buf_printf(, ", ");
+
+   if (ccd)
+   seq_buf_printf(, "Indirect branch cache disabled");
+   } else
+   seq_buf_printf(, "Vulnerable");
+
+   if (ori)
+   seq_buf_printf(, ", ori31 speculation barrier enabled");
+
+   seq_buf_printf(, "\n");
+
+   return s.len;
+}
-- 
2.14.1

[RFC PATCH 9/10] powerpc/64s: Wire up cpu_show_spectre_v1()

[Michael Ellerman]

Add a definition for cpu_show_spectre_v1() to override the generic
version. Currently this just prints "Not affected" or "Vulnerable"
based on the firmware flag.

Although the kernel does have array_index_nospec() in a few places, we
haven't yet audited all the powerpc code to see where it's necessary,
so for now we don't list that as a mitigation.

Signed-off-by: Michael Ellerman 
---
 arch/powerpc/kernel/security.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index 865db6f8bcca..0eace3cac818 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -50,3 +50,11 @@ ssize_t cpu_show_meltdown(struct device *dev, struct 
device_attribute *attr, cha
 
return sprintf(buf, "Vulnerable\n");
 }
+
+ssize_t cpu_show_spectre_v1(struct device *dev, struct device_attribute *attr, 
char *buf)
+{
+   if (!security_ftr_enabled(SEC_FTR_BNDS_CHK_SPEC_BAR))
+   return sprintf(buf, "Not affected\n");
+
+   return sprintf(buf, "Vulnerable\n");
+}
-- 
2.14.1

[RFC PATCH 8/10] powerpc/pseries: Use the security flags in pseries_setup_rfi_flush()

[Michael Ellerman]

Now that we have the security flags we can simplify the code in
pseries_setup_rfi_flush() because the security flags have pessimistic
defaults.

Signed-off-by: Michael Ellerman 
---
 arch/powerpc/platforms/pseries/setup.c | 39 ++
 1 file changed, 16 insertions(+), 23 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/setup.c 
b/arch/powerpc/platforms/pseries/setup.c
index 8ae04b586abe..2af14af6c410 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -501,38 +501,31 @@ static void pseries_setup_rfi_flush(void)
bool enable;
long rc;
 
-   /* Enable by default */
-   enable = true;
-
rc = plpar_get_cpu_characteristics();
-   if (rc == H_SUCCESS) {
+   if (rc == H_SUCCESS)
init_cpu_char_feature_flags();
 
-   types = L1D_FLUSH_NONE;
-
-   if (result.character & H_CPU_CHAR_L1D_FLUSH_TRIG2)
-   types |= L1D_FLUSH_MTTRIG;
-   if (result.character & H_CPU_CHAR_L1D_FLUSH_ORI30)
-   types |= L1D_FLUSH_ORI;
-
-   /* Use fallback if nothing set in hcall */
-   if (types == L1D_FLUSH_NONE)
-   types = L1D_FLUSH_FALLBACK;
-
-   if ((!(result.behaviour & H_CPU_BEHAV_L1D_FLUSH_PR)) ||
-   (!(result.behaviour & H_CPU_BEHAV_FAVOUR_SECURITY)))
-   enable = false;
-   } else {
-   /* Default to fallback if case hcall is not available */
-   types = L1D_FLUSH_FALLBACK;
-   }
-
/*
 * We're the guest so this doesn't apply to us, clear it to simplify
 * handling of it elsewhere.
 */
security_ftr_clear(SEC_FTR_L1D_FLUSH_HV);
 
+   types = L1D_FLUSH_NONE;
+
+   if (security_ftr_enabled(SEC_FTR_L1D_FLUSH_TRIG2))
+   types |= L1D_FLUSH_MTTRIG;
+
+   if (security_ftr_enabled(SEC_FTR_L1D_FLUSH_ORI30))
+   types |= L1D_FLUSH_ORI;
+
+   /* Use fallback if nothing set in hcall */
+   if (types == L1D_FLUSH_NONE)
+   types = L1D_FLUSH_FALLBACK;
+
+   enable = security_ftr_enabled(SEC_FTR_FAVOUR_SECURITY) && \
+security_ftr_enabled(SEC_FTR_L1D_FLUSH_PR);
+
setup_rfi_flush(types, enable);
 }
 
-- 
2.14.1

[RFC PATCH 7/10] powerpc/powernv: Use the security flags in pnv_setup_rfi_flush()

[Michael Ellerman]

Now that we have the security flags we can significantly simplify the
code in pnv_setup_rfi_flush(), because we can use the flags instead of
checking device tree properties and because the security flags have
pessimistic defaults.

Signed-off-by: Michael Ellerman 
---
 arch/powerpc/platforms/powernv/setup.c | 39 --
 1 file changed, 9 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/setup.c 
b/arch/powerpc/platforms/powernv/setup.c
index 5f242b1bab01..8f3e7a84bbf5 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -99,11 +99,10 @@ static void pnv_setup_rfi_flush(void)
 {
struct device_node *np, *fw_features;
enum l1d_flush_type type;
-   int enable;
+   bool enable;
 
/* Default to fallback in case fw-features are not available */
type = L1D_FLUSH_FALLBACK;
-   enable = 1;
 
np = of_find_node_by_name(NULL, "ibm,opal");
fw_features = of_get_child_by_name(np, "fw-features");
@@ -111,40 +110,20 @@ static void pnv_setup_rfi_flush(void)
 
if (fw_features) {
init_fw_feat_flags(fw_features);
+   of_node_put(fw_features);
 
-   np = of_get_child_by_name(fw_features, "inst-l1d-flush-trig2");
-   if (np && of_property_read_bool(np, "enabled"))
+   if (security_ftr_enabled(SEC_FTR_L1D_FLUSH_TRIG2))
type = L1D_FLUSH_MTTRIG;
 
-   of_node_put(np);
-
-   np = of_get_child_by_name(fw_features, 
"inst-l1d-flush-ori30,30,0");
-   if (np && of_property_read_bool(np, "enabled"))
+   if (security_ftr_enabled(SEC_FTR_L1D_FLUSH_ORI30))
type = L1D_FLUSH_ORI;
-
-   of_node_put(np);
-
-   /* Enable unless firmware says NOT to */
-   enable = 2;
-   np = of_get_child_by_name(fw_features, 
"needs-l1d-flush-msr-hv-1-to-0");
-   if (np && of_property_read_bool(np, "disabled"))
-   enable--;
-
-   of_node_put(np);
-
-   np = of_get_child_by_name(fw_features, 
"needs-l1d-flush-msr-pr-0-to-1");
-   if (np && of_property_read_bool(np, "disabled"))
-   enable--;
-
-   np = of_get_child_by_name(fw_features, 
"speculation-policy-favor-security");
-   if (np && of_property_read_bool(np, "disabled"))
-   enable = 0;
-
-   of_node_put(np);
-   of_node_put(fw_features);
}
 
-   setup_rfi_flush(type, enable > 0);
+   enable = security_ftr_enabled(SEC_FTR_FAVOUR_SECURITY) && \
+(security_ftr_enabled(SEC_FTR_L1D_FLUSH_PR)   || \
+ security_ftr_enabled(SEC_FTR_L1D_FLUSH_HV));
+
+   setup_rfi_flush(type, enable);
 }
 
 static void __init pnv_setup_arch(void)
-- 
2.14.1

Re: [PATCH] KVM: PPC: Book3S HV: Fix guest time accounting with VIRT_CPU_ACCOUNTING_GEN

[Paolo Bonzini]

On 02/03/2018 11:51, Laurent Vivier wrote:
> Since commit 8b24e69fc47e ("KVM: PPC: Book3S HV: Close race with testing
> for signals on guest entry"), if CONFIG_VIRT_CPU_ACCOUNTING_GEN is set, the
> guest time is not accounted to guest time and user time, but instead to
> system time.
> 
> This is because guest_enter()/guest_exit() are called while interrupts
> are disabled and the tick counter cannot be updated between them.
> 
> To fix that, move guest_exit() after local_irq_enable(), and as
> guest_enter() is called with IRQ disabled, calls guest_enter_irqoff()
> instead.
> 
> Fixes: 8b24e69fc47e
> ("KVM: PPC: Book3S HV: Close race with testing for signals on guest entry")
> Signed-off-by: Laurent Vivier 

Reviewed-by: Paolo Bonzini 

> ---
>  arch/powerpc/kvm/book3s_hv.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 89707354c2ef..8274c2807202 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -2885,7 +2885,7 @@ static noinline void kvmppc_run_core(struct 
> kvmppc_vcore *vc)
>*/
>   trace_hardirqs_on();
>  
> - guest_enter();
> + guest_enter_irqoff();
>  
>   srcu_idx = srcu_read_lock(>kvm->srcu);
>  
> @@ -2893,8 +2893,6 @@ static noinline void kvmppc_run_core(struct 
> kvmppc_vcore *vc)
>  
>   srcu_read_unlock(>kvm->srcu, srcu_idx);
>  
> - guest_exit();
> -
>   trace_hardirqs_off();
>   set_irq_happened(trap);
>  
> @@ -2937,6 +2935,7 @@ static noinline void kvmppc_run_core(struct 
> kvmppc_vcore *vc)
>   kvmppc_set_host_core(pcpu);
>  
>   local_irq_enable();
> + guest_exit();
>  
>   /* Let secondaries go back to the offline loop */
>   for (i = 0; i < controlled_threads; ++i) {
> 

[PATCH] KVM: PPC: Book3S HV: Fix guest time accounting with VIRT_CPU_ACCOUNTING_GEN

[Laurent Vivier]

Since commit 8b24e69fc47e ("KVM: PPC: Book3S HV: Close race with testing
for signals on guest entry"), if CONFIG_VIRT_CPU_ACCOUNTING_GEN is set, the
guest time is not accounted to guest time and user time, but instead to
system time.

This is because guest_enter()/guest_exit() are called while interrupts
are disabled and the tick counter cannot be updated between them.

To fix that, move guest_exit() after local_irq_enable(), and as
guest_enter() is called with IRQ disabled, calls guest_enter_irqoff()
instead.

Fixes: 8b24e69fc47e
("KVM: PPC: Book3S HV: Close race with testing for signals on guest entry")
Signed-off-by: Laurent Vivier 
---
 arch/powerpc/kvm/book3s_hv.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 89707354c2ef..8274c2807202 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2885,7 +2885,7 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore 
*vc)
 */
trace_hardirqs_on();
 
-   guest_enter();
+   guest_enter_irqoff();
 
srcu_idx = srcu_read_lock(>kvm->srcu);
 
@@ -2893,8 +2893,6 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore 
*vc)
 
srcu_read_unlock(>kvm->srcu, srcu_idx);
 
-   guest_exit();
-
trace_hardirqs_off();
set_irq_happened(trap);
 
@@ -2937,6 +2935,7 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore 
*vc)
kvmppc_set_host_core(pcpu);
 
local_irq_enable();
+   guest_exit();
 
/* Let secondaries go back to the offline loop */
for (i = 0; i < controlled_threads; ++i) {
-- 
2.14.3

[PATCH v10 2/2] cxl: read PHB indications from the device tree

[Philippe Bergheaud]

Configure the P9 XSL_DSNCTL register with PHB indications found
in the device tree, or else use legacy hard-coded values.

Signed-off-by: Philippe Bergheaud 
Reviewed-by: Frederic Barrat 
---
Changelog:

v2: New patch. Use the new device tree property "ibm,phb-indications".

v3: No change.

v4: No functional change.
Drop cosmetic fix in comment.

v5: get_phb_indications():
  - make static variables local to function.
  - return static variable values by arguments.

v6: get_phb_indications():
  - acquire a mutex before setting the phb indications.

v7: get_phb_indications():
cxl_get_xsl9_dsnctl():
  - return -ENODEV instead of -1.

v8: get_phb_indications():
  - stay on the safe side: acquire the mutex unconditionally

v9,v10: No change.
---
 drivers/misc/cxl/cxl.h|  2 +-
 drivers/misc/cxl/cxllib.c |  2 +-
 drivers/misc/cxl/pci.c| 48 ++-
 3 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 4f015da78f28..a7689944b351 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -1065,7 +1065,7 @@ int cxl_psl_purge(struct cxl_afu *afu);
 int cxl_calc_capp_routing(struct pci_dev *dev, u64 *chipid,
  u32 *phb_index, u64 *capp_unit_id);
 int cxl_slot_is_switched(struct pci_dev *dev);
-int cxl_get_xsl9_dsnctl(u64 capp_unit_id, u64 *reg);
+int cxl_get_xsl9_dsnctl(struct pci_dev *dev, u64 capp_unit_id, u64 *reg);
 u64 cxl_calculate_sr(bool master, bool kernel, bool real_mode, bool p9);
 
 void cxl_native_irq_dump_regs_psl9(struct cxl_context *ctx);
diff --git a/drivers/misc/cxl/cxllib.c b/drivers/misc/cxl/cxllib.c
index 30ccba436b3b..bea1eb004b49 100644
--- a/drivers/misc/cxl/cxllib.c
+++ b/drivers/misc/cxl/cxllib.c
@@ -99,7 +99,7 @@ int cxllib_get_xsl_config(struct pci_dev *dev, struct 
cxllib_xsl_config *cfg)
if (rc)
return rc;
 
-   rc = cxl_get_xsl9_dsnctl(capp_unit_id, >dsnctl);
+   rc = cxl_get_xsl9_dsnctl(dev, capp_unit_id, >dsnctl);
if (rc)
return rc;
if (cpu_has_feature(CPU_FTR_POWER9_DD1)) {
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 758842f65a1b..8d179e64a296 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -407,21 +407,59 @@ int cxl_calc_capp_routing(struct pci_dev *dev, u64 
*chipid,
return 0;
 }
 
-int cxl_get_xsl9_dsnctl(u64 capp_unit_id, u64 *reg)
+static DEFINE_MUTEX(indications_mutex);
+
+static int get_phb_indications(struct pci_dev *dev, u64 *capiind, u64 *asnind,
+  u64 *nbwind)
+{
+   static u64 nbw, asn, capi = 0;
+   struct device_node *np;
+   const __be32 *prop;
+
+   mutex_lock(_mutex);
+   if (!capi) {
+   if (!(np = pnv_pci_get_phb_node(dev))) {
+   mutex_unlock(_mutex);
+   return -ENODEV;
+   }
+
+   prop = of_get_property(np, "ibm,phb-indications", NULL);
+   if (!prop) {
+   nbw = 0x0300UL; /* legacy values */
+   asn = 0x0400UL;
+   capi = 0x0200UL;
+   } else {
+   nbw = (u64)be32_to_cpu(prop[2]);
+   asn = (u64)be32_to_cpu(prop[1]);
+   capi = (u64)be32_to_cpu(prop[0]);
+   }
+   of_node_put(np);
+   }
+   *capiind = capi;
+   *asnind = asn;
+   *nbwind = nbw;
+   mutex_unlock(_mutex);
+   return 0;
+}
+
+int cxl_get_xsl9_dsnctl(struct pci_dev *dev, u64 capp_unit_id, u64 *reg)
 {
u64 xsl_dsnctl;
+   u64 capiind, asnind, nbwind;
 
/*
 * CAPI Identifier bits [0:7]
 * bit 61:60 MSI bits --> 0
 * bit 59 TVT selector --> 0
 */
+   if (get_phb_indications(dev, , , ))
+   return -ENODEV;
 
/*
 * Tell XSL where to route data to.
 * The field chipid should match the PHB CAPI_CMPM register
 */
-   xsl_dsnctl = ((u64)0x2 << (63-7)); /* Bit 57 */
+   xsl_dsnctl = (capiind << (63-15)); /* Bit 57 */
xsl_dsnctl |= (capp_unit_id << (63-15));
 
/* nMMU_ID Defaults to: b’01001’*/
@@ -435,14 +473,14 @@ int cxl_get_xsl9_dsnctl(u64 capp_unit_id, u64 *reg)
 * nbwind=0x03, bits [57:58], must include capi indicator.
 * Not supported on P9 DD1.
 */
-   xsl_dsnctl |= ((u64)0x03 << (63-47));
+   xsl_dsnctl |= (nbwind << (63-55));
 
/*
 * Upper 16b address bits of ASB_Notify messages sent to the
 * system. Need to match the PHB’s ASN Compare/Mask Register.
 * Not supported on P9 DD1.
 */
-   xsl_dsnctl |= ((u64)0x04 << (63-55));
+   xsl_dsnctl |= asnind;
  

[PATCH v10 1/2] powerpc/powernv: Enable tunneled operations

[Philippe Bergheaud]

P9 supports PCI tunneled operations (atomics and as_notify). This
patch adds support for tunneled operations on powernv, with a new
API, to be called by device drivers:

pnv_pci_enable_tunnel()
   Enable tunnel operations, tell driver the 16-bit ASN indication
   used by kernel.

pnv_pci_disable_tunnel()
   Disable tunnel operations.

pnv_pci_set_tunnel_bar()
   Tell kernel the Tunnel BAR Response address used by driver.
   This function uses two new OPAL calls, as the PBCQ Tunnel BAR
   register is configured by skiboot.

pnv_pci_get_as_notify_info()
   Return the ASN info of the thread to be woken up.

Signed-off-by: Philippe Bergheaud 
Reviewed-by: Frederic Barrat 
---
Changelog:

v2: Do not set the ASN indication. Get it from the device tree.

v3: Make pnv_pci_get_phb_node() available when compiling without cxl.

v4: Add pnv_pci_get_as_notify_info().
Rebase opal call numbers on skiboot 5.9.6.

v5: pnv_pci_get_tunnel_ind():
  - fix node reference count
pnv_pci_get_as_notify_info():
  - fail if task == NULL
  - read pid from mm->context.id
  - explain that thread.tidr require CONFIG_PPC64

v6: pnv_pci_get_tunnel_ind():
  - check if radix is enabled, or else return an error
pnv_pci_get_as_notify_info():
  - remove a capi-specific comment, irrelevant for pci

v7: pnv_pci_set_tunnel_bar():
  - setting the tunnel bar more than once with the same value
is not an error

v8: No change

v9: Rename pnv_pci_get_tunnel_ind() into pnv_pci_enable_tunnel():
  - Increase real window size to accept as_notify messages.
New api pnv_pci_disable_tunnel():
  - Restore real window size to its default value.
Adjust opal call numbers.

v10: Adjust opal call numbers to their final values.
---
 arch/powerpc/include/asm/opal-api.h|   4 +-
 arch/powerpc/include/asm/opal.h|   2 +
 arch/powerpc/include/asm/pnv-pci.h |   6 ++
 arch/powerpc/platforms/powernv/opal-wrappers.S |   2 +
 arch/powerpc/platforms/powernv/pci-cxl.c   |   8 --
 arch/powerpc/platforms/powernv/pci.c   | 135 +
 6 files changed, 148 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/include/asm/opal-api.h 
b/arch/powerpc/include/asm/opal-api.h
index 94bd1bf2c873..d886a5b7ff21 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -204,7 +204,9 @@
 #define OPAL_NPU_SPA_SETUP 159
 #define OPAL_NPU_SPA_CLEAR_CACHE   160
 #define OPAL_NPU_TL_SET161
-#define OPAL_LAST  161
+#define OPAL_PCI_GET_PBCQ_TUNNEL_BAR   164
+#define OPAL_PCI_SET_PBCQ_TUNNEL_BAR   165
+#define OPAL_LAST  165
 
 /* Device tree flags */
 
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 12e70fb58700..dde60089d0d4 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -204,6 +204,8 @@ int64_t opal_unregister_dump_region(uint32_t id);
 int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val);
 int64_t opal_config_cpu_idle_state(uint64_t state, uint64_t flag);
 int64_t opal_pci_set_phb_cxl_mode(uint64_t phb_id, uint64_t mode, uint64_t 
pe_number);
+int64_t opal_pci_get_pbcq_tunnel_bar(uint64_t phb_id, uint64_t *addr);
+int64_t opal_pci_set_pbcq_tunnel_bar(uint64_t phb_id, uint64_t addr);
 int64_t opal_ipmi_send(uint64_t interface, struct opal_ipmi_msg *msg,
uint64_t msg_len);
 int64_t opal_ipmi_recv(uint64_t interface, struct opal_ipmi_msg *msg,
diff --git a/arch/powerpc/include/asm/pnv-pci.h 
b/arch/powerpc/include/asm/pnv-pci.h
index 3e5cf251ad9a..d2d8c28db336 100644
--- a/arch/powerpc/include/asm/pnv-pci.h
+++ b/arch/powerpc/include/asm/pnv-pci.h
@@ -29,6 +29,12 @@ extern int pnv_pci_set_power_state(uint64_t id, uint8_t 
state,
 extern int pnv_pci_set_p2p(struct pci_dev *initiator, struct pci_dev *target,
   u64 desc);
 
+extern int pnv_pci_enable_tunnel(struct pci_dev *dev, uint64_t *asnind);
+extern int pnv_pci_disable_tunnel(struct pci_dev *dev);
+extern int pnv_pci_set_tunnel_bar(struct pci_dev *dev, uint64_t addr,
+ int enable);
+extern int pnv_pci_get_as_notify_info(struct task_struct *task, u32 *lpid,
+ u32 *pid, u32 *tid);
 int pnv_phb_to_cxl_mode(struct pci_dev *dev, uint64_t mode);
 int pnv_cxl_ioda_msi_setup(struct pci_dev *dev, unsigned int hwirq,
   unsigned int virq);
diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S 
b/arch/powerpc/platforms/powernv/opal-wrappers.S
index 1b2936ba6040..3da30c2f26b4 100644
--- a/arch/powerpc/platforms/powernv/opal-wrappers.S
+++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
@@ -323,3 +323,5 @@ OPAL_CALL(opal_sensor_group_clear,  

Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig

[Ulf Magnusson]

On Fri, Mar 02, 2018 at 10:03:26AM +0100, Ulf Magnusson wrote:
> On Fri, Mar 02, 2018 at 02:50:39PM +0900, Masahiro Yamada wrote:
> > 2018-02-22 6:39 GMT+09:00 Ulf Magnusson :
> > > On Wed, Feb 21, 2018 at 09:57:03PM +0900, Masahiro Yamada wrote:
> > >> 2018-02-21 19:52 GMT+09:00 Arnd Bergmann :
> > >> > On Wed, Feb 21, 2018 at 11:20 AM, Masahiro Yamada
> > >> >  wrote:
> > >> >> 2018-02-21 18:56 GMT+09:00 Arnd Bergmann :
> > >> >>> On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada
> > >> >>>  wrote:
> > >>  2018-02-20 0:18 GMT+09:00 Ulf Magnusson :
> > >> >>
> > >> >> Let me clarify my concern.
> > >> >>
> > >> >> When we test the compiler flag, is there a case
> > >> >> where a particular flag depends on -m{32,64} ?
> > >> >>
> > >> >> For example, is there a compiler that supports -fstack-protector
> > >> >> for 64bit mode, but unsupports it for 32bit mode?
> > >> >>
> > >> >>   $(cc-option -m32) ->  y
> > >> >>   $(cc-option -m64) ->  y
> > >> >>   $(cc-option -fstack-protector)->  y
> > >> >>   $(cc-option -m32 -fstack-protector)   ->  n
> > >> >>   $(cc-option -m64 -fstack-protector)   ->  y
> > >> >>
> > >> >> I guess this is unlikely to happen,
> > >> >> but I am not whether it is zero possibility.
> > >> >>
> > >> >> If this could happen,
> > >> >> $(cc-option ) must be evaluated together with
> > >> >> correct bi-arch option (either -m32 or -m64).
> > >> >>
> > >> >>
> > >> >> Currently, -m32/-m64 is specified in Makefile,
> > >> >> but we are moving compiler tests to Kconfig
> > >> >> and, CONFIG_64BIT can be dynamically toggled in Kconfig.
> > >> >
> > >> > I don't think it can happen for this particular combination (stack 
> > >> > protector
> > >> > and word size), but I'm sure we'll eventually run into options that
> > >> > need to be tested in combination. For the current CFLAGS_KERNEL
> > >> > setting, we definitely have the case of needing the variables to be
> > >> > evaluated in a specific order.
> > >> >
> > >>
> > >>
> > >>
> > >>
> > >> I was thinking of how we can handle complex cases
> > >> in the current approach.
> > >>
> > >>
> > >>
> > >> (Case 1)
> > >>
> > >> Compiler flag -foo and -bar interacts, so
> > >> we also need to check the combination of the two.
> > >>
> > >>
> > >> config CC_HAS_FOO
> > >> def_bool $(cc-option -foo)
> > >>
> > >> config CC_HAS_BAR
> > >> def_bool $(cc-option -bar)
> > >>
> > >> config CC_HAS_FOO_WITH_BAR
> > >> def_bool $(cc-option -foo -bar)
> > >>
> > >>
> > >>
> > >> (Case 2)
> > >> Compiler flag -foo is sensitive to word-size.
> > >> So, we need to test this option together with -m32/-m64.
> > >> User can toggle CONFIG_64BIT, like i386/x86_64.
> > >>
> > >>
> > >> config CC_NEEDS_M64
> > >>   def_bool $(cc-option -m64) && 64BIT
> > >>
> > >> config CC_NEEDS_M32
> > >>   def_bool $(cc-option -m32) && !64BIT
> > >>
> > >> config CC_HAS_FOO
> > >>  bool
> > >>  default $(cc-option -m64 -foo) if CC_NEEDS_M64
> > >>  default $(cc-option -m32 -foo) if CC_NEEDS_M32
> > >>  default $(cc-option -foo)
> > >>
> > >>
> > >>
> > >> (Case 3)
> > >> Compiler flag -foo is sensitive to endian-ness.
> > >>
> > >>
> > >> config CC_NEEDS_BIG_ENDIAN
> > >>   def_bool $(cc-option -mbig-endian) && CPU_BIG_ENDIAN
> > >>
> > >> config CC_NEEDS_LITTLE_ENDIAN
> > >>   def_bool $(cc-option -mlittle-endian) && CPU_LITTLE_ENDIAN
> > >>
> > >> config CC_HAS_FOO
> > >>  bool
> > >>  default $(cc-option -mbig-endian -foo) if CC_NEEDS_BIG_ENDIAN
> > >>  default $(cc-option -mlittle-endian -foo) if 
> > >> CC_NEEDS_LITTLE_ENDIAN
> > >>  default $(cc-option -foo)
> > >>
> > >>
> > >>
> > >>
> > >> Hmm, I think I can implement those somehow.
> > >> But, I hope we do not have many instances like this...
> > >>
> > >>
> > >> If you know more naive cases, please share your knowledge.
> > >>
> > >> Thanks!
> > >>
> > >>
> > >> --
> > >> Best Regards
> > >> Masahiro Yamada
> > >
> > > Would get pretty bad if a test needs to consider multiple symbols.
> > > Exponential explosion there...
> > >
> > >
> > > I thought some more about the implementation of dynamic (post-parsing)
> > > functions to see how bad it would get with the current implementation.
> > >
> > > Some background on how things work now:
> > >
> > >   1. All expression operands in Kconfig are symbols.
> > >
> > >   2. Returning '$ENV' or '$(fn foo)' as a T_WORD during parsing gets
> > >  you symbols with those strings as names and S_UNKNOWN type (because
> > >  they act like references to undefined symbols).
> > >
> > >   3. For "foo-$(fn foo)", you also get a symbol with that string as its
> > >  name and S_UNKNOWN type (stored among the SYMBOL_CONST symbols)
> > >
> > >   4. Symbols with S_UNKNOWN type get 

Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig

[Ulf Magnusson]

On Fri, Mar 02, 2018 at 02:50:39PM +0900, Masahiro Yamada wrote:
> 2018-02-22 6:39 GMT+09:00 Ulf Magnusson :
> > On Wed, Feb 21, 2018 at 09:57:03PM +0900, Masahiro Yamada wrote:
> >> 2018-02-21 19:52 GMT+09:00 Arnd Bergmann :
> >> > On Wed, Feb 21, 2018 at 11:20 AM, Masahiro Yamada
> >> >  wrote:
> >> >> 2018-02-21 18:56 GMT+09:00 Arnd Bergmann :
> >> >>> On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada
> >> >>>  wrote:
> >>  2018-02-20 0:18 GMT+09:00 Ulf Magnusson :
> >> >>
> >> >> Let me clarify my concern.
> >> >>
> >> >> When we test the compiler flag, is there a case
> >> >> where a particular flag depends on -m{32,64} ?
> >> >>
> >> >> For example, is there a compiler that supports -fstack-protector
> >> >> for 64bit mode, but unsupports it for 32bit mode?
> >> >>
> >> >>   $(cc-option -m32) ->  y
> >> >>   $(cc-option -m64) ->  y
> >> >>   $(cc-option -fstack-protector)->  y
> >> >>   $(cc-option -m32 -fstack-protector)   ->  n
> >> >>   $(cc-option -m64 -fstack-protector)   ->  y
> >> >>
> >> >> I guess this is unlikely to happen,
> >> >> but I am not whether it is zero possibility.
> >> >>
> >> >> If this could happen,
> >> >> $(cc-option ) must be evaluated together with
> >> >> correct bi-arch option (either -m32 or -m64).
> >> >>
> >> >>
> >> >> Currently, -m32/-m64 is specified in Makefile,
> >> >> but we are moving compiler tests to Kconfig
> >> >> and, CONFIG_64BIT can be dynamically toggled in Kconfig.
> >> >
> >> > I don't think it can happen for this particular combination (stack 
> >> > protector
> >> > and word size), but I'm sure we'll eventually run into options that
> >> > need to be tested in combination. For the current CFLAGS_KERNEL
> >> > setting, we definitely have the case of needing the variables to be
> >> > evaluated in a specific order.
> >> >
> >>
> >>
> >>
> >>
> >> I was thinking of how we can handle complex cases
> >> in the current approach.
> >>
> >>
> >>
> >> (Case 1)
> >>
> >> Compiler flag -foo and -bar interacts, so
> >> we also need to check the combination of the two.
> >>
> >>
> >> config CC_HAS_FOO
> >> def_bool $(cc-option -foo)
> >>
> >> config CC_HAS_BAR
> >> def_bool $(cc-option -bar)
> >>
> >> config CC_HAS_FOO_WITH_BAR
> >> def_bool $(cc-option -foo -bar)
> >>
> >>
> >>
> >> (Case 2)
> >> Compiler flag -foo is sensitive to word-size.
> >> So, we need to test this option together with -m32/-m64.
> >> User can toggle CONFIG_64BIT, like i386/x86_64.
> >>
> >>
> >> config CC_NEEDS_M64
> >>   def_bool $(cc-option -m64) && 64BIT
> >>
> >> config CC_NEEDS_M32
> >>   def_bool $(cc-option -m32) && !64BIT
> >>
> >> config CC_HAS_FOO
> >>  bool
> >>  default $(cc-option -m64 -foo) if CC_NEEDS_M64
> >>  default $(cc-option -m32 -foo) if CC_NEEDS_M32
> >>  default $(cc-option -foo)
> >>
> >>
> >>
> >> (Case 3)
> >> Compiler flag -foo is sensitive to endian-ness.
> >>
> >>
> >> config CC_NEEDS_BIG_ENDIAN
> >>   def_bool $(cc-option -mbig-endian) && CPU_BIG_ENDIAN
> >>
> >> config CC_NEEDS_LITTLE_ENDIAN
> >>   def_bool $(cc-option -mlittle-endian) && CPU_LITTLE_ENDIAN
> >>
> >> config CC_HAS_FOO
> >>  bool
> >>  default $(cc-option -mbig-endian -foo) if CC_NEEDS_BIG_ENDIAN
> >>  default $(cc-option -mlittle-endian -foo) if 
> >> CC_NEEDS_LITTLE_ENDIAN
> >>  default $(cc-option -foo)
> >>
> >>
> >>
> >>
> >> Hmm, I think I can implement those somehow.
> >> But, I hope we do not have many instances like this...
> >>
> >>
> >> If you know more naive cases, please share your knowledge.
> >>
> >> Thanks!
> >>
> >>
> >> --
> >> Best Regards
> >> Masahiro Yamada
> >
> > Would get pretty bad if a test needs to consider multiple symbols.
> > Exponential explosion there...
> >
> >
> > I thought some more about the implementation of dynamic (post-parsing)
> > functions to see how bad it would get with the current implementation.
> >
> > Some background on how things work now:
> >
> >   1. All expression operands in Kconfig are symbols.
> >
> >   2. Returning '$ENV' or '$(fn foo)' as a T_WORD during parsing gets
> >  you symbols with those strings as names and S_UNKNOWN type (because
> >  they act like references to undefined symbols).
> >
> >   3. For "foo-$(fn foo)", you also get a symbol with that string as its
> >  name and S_UNKNOWN type (stored among the SYMBOL_CONST symbols)
> >
> >   4. Symbols with S_UNKNOWN type get their name as their string value,
> >  and the tristate value n.
> >
> > So, if you do string expansion on the names of symbols with S_UNKNOWN
> > type in sym_calc_value(), you're almost there with the current
> > implementation, except for the tristate case.
> >
> > Maybe you could set the tristate value of S_UNKNOWN symbols depending on
> >