Re: [PATCH 2/2] ppc/shutdown: Implement machine_{halt,restart}()

[Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>]

Hi Andrew,

Overall this looks good, I just have one comment.

On 7/5/24 1:23 PM, Andrew Cooper wrote:
> diff --git a/xen/arch/ppc/shutdown.c b/xen/arch/ppc/shutdown.c
> new file mode 100644
> index 000000000000..c4b00e57b13a
> --- /dev/null
> +++ b/xen/arch/ppc/shutdown.c
> @@ -0,0 +1,43 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#include <xen/shutdown.h>
> +
> +#include <asm/opal-api.h>
> +
> +int64_t opal_cec_power_down(uint64_t request);
> +int64_t opal_cec_reboot(void);
> +int64_t opal_poll_events(uint64_t *outstanding_event_mask);
> +
> +void machine_halt(void)
> +{
> +    int rc;
> +

Before the opal_cec_{power_down,reboot} calls are made, any interupts
reserved for OPAL to handle need to be masked off.

Since we don't have any set up at this point, I would just add a TODO
comment like:

/* TODO: mask any OPAL IRQs before shutting down */

> +    do {
> +        rc = opal_cec_power_down(0);
> +
> +        if ( rc == OPAL_BUSY_EVENT )
> +            opal_poll_events(NULL);
> +
> +    } while ( rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT );
> +
> +    for ( ;; )
> +        opal_poll_events(NULL);
> +}
> +
> +void machine_restart(unsigned int delay_millisecs)
> +{
> +    int rc;
> +
> +    /* TODO: mdelay(delay_millisecs); */
> +

Ditto.

> +    do {
> +        rc = opal_cec_reboot();
> +
> +        if ( rc == OPAL_BUSY_EVENT )
> +            opal_poll_events(NULL);
> +
> +    } while ( rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT );
> +
> +    for ( ;; )
> +        opal_poll_events(NULL);
> +}
> +

Other than that, looks great.

Reviewed-by: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>

Thanks,
Shawn