On 13.03.2018 13:01, Janosch Frank wrote:
> Storage keys are not used by Linux anymore, so let's show them some
> love and test if the basics still work.
>
> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxxxxxxx>
> ---
> lib/s390x/asm/mem.h | 61 ++++++++++++++++++++++++++++++++++
> s390x/Makefile | 1 +
> s390x/skey.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> s390x/unittests.cfg | 3 ++
> 4 files changed, 159 insertions(+)
> create mode 100644 lib/s390x/asm/mem.h
> create mode 100644 s390x/skey.c
>
> diff --git a/lib/s390x/asm/mem.h b/lib/s390x/asm/mem.h
> new file mode 100644
> index 0000000..28772b0
> --- /dev/null
> +++ b/lib/s390x/asm/mem.h
> @@ -0,0 +1,61 @@
> +/*
> + * Physical memory management related functions and definitions.
> + *
> + * Copyright IBM Corp. 2017
2018 ?
> + * Author(s): Janosch Frank <frankja@xxxxxxxxxx>
> + *
> + * This code is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Library General Public License version 2.
> + */
> +#ifndef _ASM_S390_MEM_H
> +#define _ASM_S390_MEM_H
> +
> +union skey {
> + struct {
> + uint8_t acc : 4;
> + uint8_t fp : 1;
> + uint8_t rf : 1;
> + uint8_t ch : 1;
> + uint8_t pad : 1;
> + } str;
> + uint8_t val;
> +};
> +
> +static inline void set_storage_key(unsigned long addr,
> + unsigned char skey,
> + int nq)
I did not spot any occurance of set_storage_key(..., true) in this
patch, so do you really need this parameter?
> +{
> + if (nq && test_facility(14))
I think you could simply drop the test_facility check here since the bit
is ignored accoring to the PoP if the facility is not installed.
> + asm volatile(".insn rrf,0xb22b0000,%0,%1,8,0"
> + : : "d" (skey), "a" (addr));
> + else
> + asm volatile("sske %0,%1" : : "d" (skey), "a" (addr));
> +}
> +
> +static inline unsigned long set_storage_key_mb(unsigned long addr,
> + unsigned char skey)
> +{
> + if (!test_facility(8))
> + return 0;
You check that already in test_set_mb() ... so I'd either remove this
if-statement or turn it into a assert(test_facility(8)).
> + /* As we only have one cpu and no concurrent skey changes,
> + * we're able to use the non-quescing option (if available) to
> + * speed things up a little.
> + */
> + if (test_facility(14))
Again, no need to explicitly check for that facility here - the bit is
ignored if the facility is not available.
> + asm volatile(".insn rrf,0xb22b0000,%[skey],%[addr],9,0"
> + : [addr] "+a" (addr) : [skey] "d" (skey));
> + else
> + asm volatile(".insn rrf,0xb22b0000,%[skey],%[addr],1,0"
> + : [addr] "+a" (addr) : [skey] "d" (skey));
> + return addr;
> +}
> +
> +static inline unsigned char get_storage_key(unsigned long addr)
> +{
> + unsigned char skey;
> +
> + asm volatile("iske %0,%1" : "=d" (skey) : "a" (addr));
> + return skey;
> +}
> +#endif
The other parts of the patch look fine to me.
Thomas
