Re: [PATCH v2 6/8] vpci/rebar: Remove registers when init_rebar() fails

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

On Wed, Apr 09, 2025 at 02:45:26PM +0800, Jiqian Chen wrote:
> When init_rebar() fails, the previous new changes will hide Rebar
> capability, it can't rely on vpci_deassign_device() to remove all
> Rebar related registers anymore, those registers must be removed
> in failure path of init_rebar.
> 
> To do that, call vpci_remove_registers() to remove all possible
> registered registers in the failure patch.
> 
> Signed-off-by: Jiqian Chen <Jiqian.Chen@xxxxxxx>
> ---
> cc: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
> ---
> v1->v2 changes:
> * Called vpci_remove_registers() to remove all possible registered registers 
> instead of using a array to record all registered register.
> 
> Best regards,
> Jiqian Chen.
> ---
>  xen/drivers/vpci/rebar.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/drivers/vpci/rebar.c b/xen/drivers/vpci/rebar.c
> index 79858e5dc92f..e030937956e3 100644
> --- a/xen/drivers/vpci/rebar.c
> +++ b/xen/drivers/vpci/rebar.c
> @@ -51,6 +51,7 @@ static void cf_check rebar_ctrl_write(const struct pci_dev 
> *pdev,
>  
>  static int cf_check init_rebar(struct pci_dev *pdev)
>  {
> +    int rc = 0;
>      uint32_t ctrl;
>      unsigned int nbars;
>      unsigned int rebar_offset = pci_find_ext_capability(pdev->sbdf,
> @@ -70,7 +71,6 @@ static int cf_check init_rebar(struct pci_dev *pdev)
>      nbars = MASK_EXTR(ctrl, PCI_REBAR_CTRL_NBAR_MASK);
>      for ( unsigned int i = 0; i < nbars; i++ )
>      {
> -        int rc;
>          struct vpci_bar *bar;
>          unsigned int index;
>  
> @@ -80,7 +80,7 @@ static int cf_check init_rebar(struct pci_dev *pdev)
>          {
>              printk(XENLOG_ERR "%pd %pp: too big BAR number %u in 
> REBAR_CTRL\n",
>                     pdev->domain, &pdev->sbdf, index);
> -            continue;
> +            goto fail;
>          }
>  
>          bar = &pdev->vpci->header.bars[index];
> @@ -88,7 +88,7 @@ static int cf_check init_rebar(struct pci_dev *pdev)
>          {
>              printk(XENLOG_ERR "%pd %pp: BAR%u is not in memory space\n",
>                     pdev->domain, &pdev->sbdf, index);
> -            continue;
> +            goto fail;

I think you need to set rc to an error code for the two chunks above,
otherwise you jump into the fail label with rc == 0 AFAICT.

Thanks, Roger.