Re: [PATCH] vpci: Add resizable bar support

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

On Fri, Nov 22, 2024 at 04:04:05AM +0000, Chen, Jiqian wrote:
> On 2024/11/21 17:52, Roger Pau Monné wrote:
> > On Thu, Nov 21, 2024 at 03:05:14AM +0000, Chen, Jiqian wrote:
> >> On 2024/11/20 17:01, Roger Pau Monné wrote:
> >>> On Wed, Nov 20, 2024 at 03:01:57AM +0000, Chen, Jiqian wrote:
> >>>> The only difference between our methods is the timing of updating the 
> >>>> size.
> >>>> Yours is later than mine because you updated the size when the driver 
> >>>> re-enabled memory decoding, while I updated the size in time when driver 
> >>>> resize it.
> >>>
> >>> Indeed, my last guess is the stale cached size is somehow used in my
> >>> approach, and that leads to the failures.  One last (possibly dummy?)
> >>> thing to try might be to use your patch to detect writes to the resize
> >>> control register, but update the BAR sizes in modify_bars(), while
> >>> keeping the traces of when the operations happen.
> >>>
> >> This can work, combine our method, use my patch to detect and write the 
> >> size into hardware register, and use your patch to update bar[i].size in 
> >> modify_bars().
> >> Attached the combined patch and the xl dmesg.
> > 
> > This is even weirder, so the attached patch works fine? 
> Yes, it works fine.
> And I will double check.
> 
> > The only difference with my proposal is that you trap the CTRL registers, 
> > but
> > the sizing is still done in modify_bars().
> > 
> > What happens if (based on the attached patch) you change
> > rebar_ctrl_write() to:
> > 
> > static void cf_check rebar_ctrl_write(const struct pci_dev *pdev,
> >                                       unsigned int reg,
> >                                       uint32_t val,
> >                                       void *data)
> > {
> >     pci_conf_write32(pdev->sbdf, reg, val);
> > }
> Will try.
> 
> > 
> > And if you don't trap any PCI_REBAR_CTRL at all?
> What do you mean? If I don't trap any rebar_ctrl, how can I call 
> rebar_ctrl_write?

Well, that's part of the question, is just trapping PCI_REBAR_CTRL
enough to make this work?

At the moment it's unclear to me what makes your approach work and not
mine.  And I would like to understand why your code works, otherwise
I fear I'm not understanding how the capability works, and hence our
support for it might not be reliable.

Thanks, Roger.