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

[email protected]
Newsgroups org.xenproject.lists.xen-devel
Message-ID <anYfrNnXTsWJJ5kH@kraken>
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
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.