Re: [PATCH 1/3] Setup AMOR in HV mode

["Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>]

Balbir Singh <bsingharora@gmail.com> writes:

> AMOR should be setup in HV mode, we set it up once
> and let the generic kernel handle IAMR. This patch is
> used to enable storage keys in a following patch as
> defined in ISA 3
>
> Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> ---
>  arch/powerpc/mm/pgtable-radix.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/arch/powerpc/mm/pgtable-radix.c b/arch/powerpc/mm/pgtable-radix.c
> index ed7bddc..0fdd8ed 100644
> --- a/arch/powerpc/mm/pgtable-radix.c
> +++ b/arch/powerpc/mm/pgtable-radix.c
> @@ -320,6 +320,25 @@ static void update_hid_for_radix(void)
>  		cpu_relax();
>  }
>  
> +/*
> + * In HV mode, we init AMOR so that the hypervisor
> + * and guest can setup IMAR, enable key 0 and set
> + * it to 1
> + * AMOR = 1100....00 (Mask for key 0 is 11)
> + */
> +static void __init radix_init_amor(void)
> +{
> +	unsigned long amor_mask = 0xc000000000000000;
> +	unsigned long amor = mfspr(SPRN_AMOR);
> +
> +	if (cpu_has_feature(CPU_FTR_POWER9_DD1))
> +		return;

Why is DD1 excluded ?

> +
> +	amor |= amor_mask;
> +

Can't we just init it with the constant 0xc000000000000000;
I can understand kvm code wanting to do the '|' above, but does the host
init code need to look at the previous value there ?


> +	mtspr(SPRN_AMOR, amor);
> +}
> +
>  void __init radix__early_init_mmu(void)
>  {
>  	unsigned long lpcr;
> @@ -376,6 +395,7 @@ void __init radix__early_init_mmu(void)
>  		lpcr = mfspr(SPRN_LPCR);
>  		mtspr(SPRN_LPCR, lpcr | LPCR_UPRT | LPCR_HR);
>  		radix_init_partition_table();
> +		radix_init_amor();
>  	}
>  
>  	radix_init_pgtable();

What about secondaries ? Don't we need to init AMOR there ?

-aneesh