Re: [Xen-devel] [PATCH] pci/msi: constify the pci_dev parameter of pci_msi_conf_write_intercept

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

On Mon, Sep 11, 2017 at 04:11:41AM -0600, Jan Beulich wrote:
> >>> On 11.09.17 at 11:16, <roger.pau@xxxxxxxxxx> wrote:
> 
> This being an x86 only change, the subject prefix would presumably
> better be "x86/msi".

Right. I guess you don't mind changing this upon commit if it gets
accepted.

> > --- a/xen/arch/x86/msi.c
> > +++ b/xen/arch/x86/msi.c
> > @@ -657,7 +657,7 @@ int msi_free_irq(struct msi_desc *entry)
> >      return 0;
> >  }
> >  
> > -static struct msi_desc *find_msi_entry(struct pci_dev *dev,
> > +static struct msi_desc *find_msi_entry(const struct pci_dev *dev,
> >                                         int irq, int cap_id)
> >  {
> 
> I certainly agree with this part, but ...
> 
> > @@ -1274,7 +1274,7 @@ void pci_cleanup_msi(struct pci_dev *pdev)
> >      msi_free_irqs(pdev);
> >  }
> >  
> > -int pci_msi_conf_write_intercept(struct pci_dev *pdev, unsigned int reg,
> > +int pci_msi_conf_write_intercept(const struct pci_dev *pdev, unsigned int 
> > reg,
> >                                   unsigned int size, uint32_t *data)
> >  {
> 
> ... I'm not so sure about this one. The function changes data
> associated with the device, and it just so happens that right
> now all such changes are confined to the separately allocated
> msix structure. Do other changes of yours depend on the
> parameter becoming const here? I'm trying to understand
> what scope an undo of this change would be, should it turn
> out necessary down the road.

This is used at:

https://lists.xenproject.org/archives/html/xen-devel/2017-08/msg01511.html

In the vpci_msix_control_write handler. We have spoken about
constifying the pci_dev of the handlers, and here the pci_dev argument
is being used to call into pci_msi_conf_write_intercept.

Now that I look again at the code, I think I can leave
pci_msi_conf_write_intercept alone, since in vpci_msix_control_write I
can also get the pci_dev from msix->pdev, and that's not constified. I
will send the find_msi_entry chunk separately, which I still think
it's worth.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel