Re: [Xen-devel] [PATCH] pci: constify domain parameter of pci_get_pdev_by_domain

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.09.17 at 16:30, <roger.pau@xxxxxxxxxx> wrote:
> On Fri, Sep 08, 2017 at 03:15:40PM +0100, Andrew Cooper wrote:
>> On 08/09/17 14:34, Roger Pau Monne wrote:
>> > While there fix the indentation.
>> >
>> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> > ---
>> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
>> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> > Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
>> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
>> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>> > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>> > Cc: Tim Deegan <tim@xxxxxxx>
>> > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
>> > ---
>> >  xen/drivers/passthrough/pci.c | 4 ++--
>> >  xen/include/xen/pci.h         | 4 ++--
>> >  2 files changed, 4 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
>> > index 74829e5748..469dfc6c3d 100644
>> > --- a/xen/drivers/passthrough/pci.c
>> > +++ b/xen/drivers/passthrough/pci.c
>> > @@ -532,8 +532,8 @@ struct pci_dev *pci_get_real_pdev(int seg, int bus, 
>> > int 
> devfn)
>> >      return pdev;
>> >  }
>> >  
>> > -struct pci_dev *pci_get_pdev_by_domain(
>> > -    struct domain *d, int seg, int bus, int devfn)
>> > +struct pci_dev *pci_get_pdev_by_domain(const struct domain *d, int seg,
>> > +                                       int bus, int devfn)
>> 
>> I know this isn't strictly related to the patch, but having 3 register
>> parameters (and load of places elsewhere) here is extremely wasteful for
>> register scheduling.  Could we introduce:
>> 
>> typedef union {
>>     uint32_t sbdf;
>>     struct {
>>         uint16_t  f : 3,  /* Function */
>>                   d : 5,  /* Device   */
>>                   b : 8;  /* Bus      */
>>         uint16_t s; /* Segment  */
>>     };
>> } pci_sbdf_t;
>> 
>> and try to start using it?
> 
> This is going to be kind of awkward to use with extended
> functions...
> 
> It's not like what we have now (devfn) is much better, but if we
> switch to something else I would like to be able to get the correct
> function value for both normal and extended function devices from the
> same field.

Why not unionize f and d with a uint8_t extfn (I'd also prefer the
other two to be named func and dev)?

Jan

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