> -----Original Message-----
> From: Schofield, Alison
> Sent: Saturday, September 8, 2018 10:38 AM
> To: dhowells@xxxxxxxxxx; tglx@xxxxxxxxxxxxx
> Cc: Huang, Kai <kai.huang@xxxxxxxxx>; Nakajima, Jun
> <jun.nakajima@xxxxxxxxx>; Shutemov, Kirill <kirill.shutemov@xxxxxxxxx>;
> Hansen, Dave <dave.hansen@xxxxxxxxx>; Sakkinen, Jarkko
> <jarkko.sakkinen@xxxxxxxxx>; jmorris@xxxxxxxxx; keyrings@xxxxxxxxxxxxxxx;
> linux-security-module@xxxxxxxxxxxxxxx; mingo@xxxxxxxxxx; hpa@xxxxxxxxx;
> x86@xxxxxxxxxx; linux-mm@xxxxxxxxx
> Subject: [RFC 10/12] x86/pconfig: Program memory encryption keys on a
> system-wide basis
>
> The kernel manages the MKTME (Multi-Key Total Memory Encryption) Keys as a
> system wide single pool of keys. The hardware, however, manages the keys on a
> per physical package basis. Each physical package maintains a key table that all
> CPU's in that package share.
>
> In order to maintain the consistent, system wide view that the kernel requires,
> program all physical packages during a key program request.
>
> Signed-off-by: Alison Schofield <alison.schofield@xxxxxxxxx>
> ---
> arch/x86/include/asm/intel_pconfig.h | 42
> ++++++++++++++++++++++++++++++------
> 1 file changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/intel_pconfig.h
> b/arch/x86/include/asm/intel_pconfig.h
> index 3cb002b1d0f9..d3bf0a297e89 100644
> --- a/arch/x86/include/asm/intel_pconfig.h
> +++ b/arch/x86/include/asm/intel_pconfig.h
> @@ -3,6 +3,7 @@
>
> #include <asm/asm.h>
> #include <asm/processor.h>
> +#include <linux/cpu.h>
>
> enum pconfig_target {
> INVALID_TARGET = 0,
> @@ -47,19 +48,48 @@ struct mktme_key_program {
> u8 key_field_2[64];
> } __packed __aligned(256);
>
> -static inline int mktme_key_program(struct mktme_key_program
> *key_program)
> +struct mktme_key_program_info {
> + struct mktme_key_program *key_program;
> + unsigned long status;
> +};
> +
> +static void mktme_package_program(void *key_program_info)
> {
> + struct mktme_key_program_info *info = key_program_info;
> unsigned long rax = MKTME_KEY_PROGRAM;
>
> + asm volatile(PCONFIG
> + : "=a" (rax), "=b" (info->key_program)
Why do we need "=b" (info->key_program)? To me PCONFIG only reads from rbx, but won't write to it.
> + : "0" (rax), "1" (info->key_program)
> + : "memory", "cc");
> +
> + if (rax != MKTME_PROG_SUCCESS)
> + WRITE_ONCE(info->status, rax);
> +}
> +
> +/*
> + * MKTME keys are managed as a system-wide single pool of keys.
> + * In the hardware, each physical package maintains a separate key
> + * table. Program all physical packages with the same key info to
> + * maintain that system-wide kernel view.
> + */
> +static inline int mktme_key_program(struct mktme_key_program
> *key_program,
> + cpumask_var_t mktme_cpumask)
> +{
> + struct mktme_key_program_info info = {
> + .key_program = key_program,
> + .status = MKTME_PROG_SUCCESS,
> + };
> +
> if (!pconfig_target_supported(MKTME_TARGET))
> return -ENXIO;
>
> - asm volatile(PCONFIG
> - : "=a" (rax), "=b" (key_program)
> - : "0" (rax), "1" (key_program)
> - : "memory", "cc");
> + get_online_cpus();
> + on_each_cpu_mask(mktme_cpumask, mktme_package_program,
> + &info, 1);
> + put_online_cpus();
What is the value of 'mktme_cpumask'? Does it only contain one core for each package?
Since we are using 'on_each_cpu_mask', I think we should make sure only one core is set for each node in 'mktme_cpumask'. Otherwise we have to deal with 'DEVICE_BUSY' case, since if one core is already in middle of PCONFIG, the other PCONFIGs on the same node would get 'DEVICE_BUSY' error, but this doesn't mean PCONFIG has failed on that node.
Thanks,
-Kai
>
> - return rax;
> + return info.status;
> }
>
> #endif /* _ASM_X86_INTEL_PCONFIG_H */
> --
> 2.14.1
