Re: [PATCH v2] acpi: reboot: log reset parameters

On Wed, Aug 05, 2026 at 12:18:33PM +0200, Jan Beulich wrote:
> On 01.08.2026 05:17, [email protected] wrote:
> > From: Denis Mukhin <[email protected]> 
> > 
> > Xen does not provide much details for system reset debugging in case
> > system reset happens via ACPI subsystem.
> > 
> > Log reset I/O address and reset value.
> > 
> > While here, add the missing default case, add breaks between case
> > statements and drop full stops in the loglines.
> > 
> > Signed-off-by: Denis Mukhin <[email protected]>
> > ---
> > - v1: https://lore.kernel.org/xen-devel/[email protected]/ 
> > - CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/2723268852
> > 
> > Changes since v1:
> > - removed wrong ASSERT_UNREACHABLE()
> 
> And you replaced it with a printk(), which I don't view as helpful. If we
> want to diagnose the address violating the spec, that should be done
> elsewhere.

Ack.

> 
> > @@ -21,17 +22,30 @@ void acpi_reboot(void)
> >  	 * on a device on bus 0. */
> >  	switch (rr->space_id) {
> >  	case ACPI_ADR_SPACE_PCI_CONFIG:
> > -		printk("Resetting with ACPI PCI RESET_REG.\n");
> > +		sbdf = PCI_SBDF(0, 0, rr->address >> 32, rr->address >> 16);
> > +		printk("Resetting with ACPI PCI %pp RESET_REG at %#lx (%#x)\n",
> > +		       &sbdf, rr->address & 0xff, reset_value);
> 
> rr->address is u64, and the code here isn't arch-specific. Yes, the file is
> built for x86 only right now, so 'l' as format modifier is kind of okay for
> the time being. But really PRIx64 would want using. (I'm sorry for not
> noticing this on v1 already.)

I had doubts on that one, but decided to go with 'l' in v2.

> 
> Preferably with that adjustment and with the excess log message dropped
> again (I can certainly do so while committing):
> Reviewed-by: Jan Beulich <[email protected]>

Thank you!

> 
> Jan
>