[Skiboot] [PATCH] xive/p9: Remove assert from xive_eq_for_target()

Greg Kurz groug at kaod.org
Fri Nov 27 19:08:07 AEDT 2020


On Fri, 27 Nov 2020 08:32:12 +0100
Cédric Le Goater <clg at kaod.org> wrote:

> XIVE VPs are structures describing the vCPUs of guests. When starting
> a guest, these are allocated and enabled and some checks are done on
> the location of the associated ENDs, which describe the event
> queues. If the block of the VP and the block of the ENDs do not match,
> the XIVE driver asserts.
> 
> Unfortunately, there is no way to check that a VP identifier is part
> of a VP block that was previously allocated and it is relatively easy
> to crash the host with a bogus VP id. That can be done with a QEMU
> hack on a machine using vsmt.
> 

Reported-by: Greg Kurz <groug at kaod.org>

:)

> Simply remove the assert, the OS should gracefully handle the error.
> 

This seems to be reasonable when xive_eq_for_target() is called from
an OPAL call since they'd all return OPAL_PARAMETER to the OS in this
case.

Some other paths maybe need more care though, eg:

xive_ipi_init()
    __xive_set_irq_config()
        xive_set_irq_targetting()
            xive_eq_for_target()

If xive_eq_for_target() fails to map the target to a valid EQ,
this ends up being ignored in xive_ipi_init() with this patch.
Is it okay ?

> Signed-off-by: Cédric Le Goater <clg at kaod.org>
> ---
>  hw/xive.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/xive.c b/hw/xive.c
> index 7d4e029f19cb..c442ea5e30ed 100644
> --- a/hw/xive.c
> +++ b/hw/xive.c
> @@ -2152,7 +2152,7 @@ static inline bool xive_eq_for_target(uint32_t target, uint8_t prio,
>  	if (eq_blk != vp_blk) {
>  		xive_err(x, "eq_blk != vp_blk (%d vs. %d) for target 0x%08x/%d\n",
>  			 eq_blk, vp_blk, target, prio);
> -		assert(false);
> +		return false;
>  	}
>  
>  	if (out_eq_blk)



More information about the Skiboot mailing list