Re: [PATCH v3 3/6] vpci: crash domain if we wasn't able to (un) map vPCI regions

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Mar 14, 2023 at 08:56:30PM +0000, Volodymyr Babchuk wrote:
> In that unlikely case, when map_range() fails to do it's job,
> domain memory mapping will be left in inconsistent state. As there is
> no easy way to remove stale p2m mapping we need to crash domain, as
> FIXME suggests.
> 
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
> 
> ---
> 
> v3:
>  - new patch
> ---
>  xen/drivers/vpci/header.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index ec2e978a4e..8319fe4c1d 100644
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -162,14 +162,11 @@ bool vpci_process_pending(struct vcpu *v)
>          rangeset_destroy(v->vpci.mem);
>          v->vpci.mem = NULL;
>          if ( rc )
> -            /*
> -             * FIXME: in case of failure remove the device from the domain.
> -             * Note that there might still be leftover mappings. While this 
> is
> -             * safe for Dom0, for DomUs the domain will likely need to be
> -             * killed in order to avoid leaking stale p2m mappings on
> -             * failure.
> -             */
> +        {
>              vpci_remove_device(v->vpci.pdev);
> +            if ( !is_hardware_domain(v->domain) )
> +                domain_crash(v->domain);

No need to remove the device if you are crashing the domain, so the
vpci_remove_device() call can be placed in the else branch of the
conditional.

Thanks, Roger.