Re: [PATCH v7 3/8] vpci: Hide legacy capability when it fails to initialize

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

On Wed, Jul 23, 2025 at 07:33:04AM +0000, Chen, Jiqian wrote:
> On 2025/7/21 23:48, Roger Pau Monné wrote:
> > On Fri, Jul 04, 2025 at 03:07:58PM +0800, Jiqian Chen wrote:
> >> +                   pdev->domain, &pdev->sbdf, type, cap, rc);
> >> +
> >> +            if ( capability->cleanup )
> >> +            {
> >> +                rc = capability->cleanup(pdev);
> >> +                if ( rc )
> >> +                {
> >> +                    printk(XENLOG_ERR "%pd %pp: clean %s cap %u fail 
> >> rc=%d\n",
> >> +                           pdev->domain, &pdev->sbdf, type, cap, rc);
> > 
> > I think it would be fine to not return error here for the hardware
> > domain, and try to mask the capability even if cleanup() has failed?
> > 
> > For the hardware domain it's likely better to not fail and attempt to
> > mask, rather than fail and then end up exposing the device without any
> > kind of vPCI mediation.  For domU the proposed behavior is fine.
> Got it.
> So here should be?
>                 if ( rc && !is_hardware_domain(pdev->domain) )
>                 {
>                     printk(XENLOG_ERR "%pd %pp: clean %s cap %u fail rc=%d\n",
>                            pdev->domain, &pdev->sbdf, type, cap, rc);
>                     return rc;
>                 }
> or
>                 if ( rc )
>                 {
>                     printk(XENLOG_ERR "%pd %pp: clean %s cap %u fail rc=%d\n",
>                            pdev->domain, &pdev->sbdf, type, cap, rc);
>                     if ( !is_hardware_domain(pdev->domain) )
>                         return rc;
>                 }

The last one will be my preference, we want to log the error, just
not forward it to the caller.

Thanks, Roger.