Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 9 Sep 2022 11:37:30 +0100
From:   Catalin Marinas <catalin.marinas@arm.com>
To:     Peter Collingbourne <pcc@google.com>
Cc:     Will Deacon <will@kernel.org>, Mark Brown <broonie@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arm64: translate register values to physical addresses
 in kernel panics
Message-ID: <YxsXamXpLLTZ1M63@arm.com>
References: <20220812183530.2261795-1-pcc@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220812183530.2261795-1-pcc@google.com>
Precedence: bulk

On Fri, Aug 12, 2022 at 11:35:30AM -0700, Peter Collingbourne wrote:
> When debugging a kernel panic it is sometimes useful to know the physical
> address of any virtual addresses stored in registers. Therefore, pass
> all register values through AT S1E1R and print the resulting PAR_EL1
> value next to the register.

I don't see much value in this but I haven't come across a use-case yet.
For page faults the kernel prints the content of the PTE and that's what
I'm usually interested in.

> Not sure if this should land in this form (I imagine there could be
> all kinds of parsers that are expecting the existing format) but
> maybe behind an option. Let me know what you think.

While that's not considered user ABI, there might be some scripts
parsing it, though I suspect they don't pay attention to the registers
(I might be wrong though).

> +static unsigned long at(unsigned long addr)
> +{
> +	unsigned long pa;
> +
> +	__asm__ __volatile__("at s1e1r, %1\n"
> +			     "mrs %0, par_el1\n"
> +			     : "=r"(pa)
> +			     : "r"(addr)
> +			     : "memory");
> +	return pa;
> +}

This should take the translation fault into account. If PAR_EL1.F is 1,
the other bits can't be treated as a physical address. Also if you want
the actual address, it's also worth masking out the non-relevant bits
from PAR_EL1 and adding the offset from 'addr' into the lower 12 bits.

>  void __show_regs(struct pt_regs *regs)
>  {
>  	int i, top_reg;
> @@ -231,10 +243,10 @@ void __show_regs(struct pt_regs *regs)
>  	i = top_reg;
>  
>  	while (i >= 0) {
> -		printk("x%-2d: %016llx", i, regs->regs[i]);
> +		printk("x%-2d: %016llx (%016llx)", i, regs->regs[i], at(regs->regs[i]));
>  
>  		while (i-- % 3)
> -			pr_cont(" x%-2d: %016llx", i, regs->regs[i]);
> +			pr_cont(" x%-2d: %016llx (%016llx)", i, regs->regs[i], at(regs->regs[i]));

How long are the lines printed here? Maybe a better option without
cluttering the register values is to do another pass through the
register and print the potential VA->PA translations (only those kernel
addresses that do not fault). If one is interested they could look them
up on the following lines.

-- 
Catalin